Move columnar runtime config helpers to Java#15032
Conversation
7bd098b to
e444cc4
Compare
6593a9f to
db64c8d
Compare
7d76368 to
e3ce48d
Compare
db64c8d to
288d815
Compare
e3ce48d to
0e024e3
Compare
037114c to
275be6e
Compare
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
0e024e3 to
9d75d70
Compare
275be6e to
66c2b1b
Compare
Greptile SummaryThis PR migrates a set of columnar runtime-configuration helpers from Scala (
Confidence Score: 4/5The change is a straightforward Scala-to-Java translation with no intended behavior change; nearly all of it is safe, with one native-resource cleanup inconsistency in GpuOrcTimezoneUtils.java worth fixing before merge. The sql-plugin-columnar/src/main/java/com/nvidia/spark/rapids/GpuOrcTimezoneUtils.java — the closeAll(ColumnView[]) method needs exception-safe iteration before merge. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before
ScalaEC[ExecutorCache.scala\nlazy val]
ScalaORC[GpuOrcTimezoneUtils.scala\nScala object]
ScalaCC[CombineConf / AutoCloseableTargetSize\nScala case classes in sql-plugin]
end
subgraph After["After (sql-plugin-columnar Java)"]
JavaEC[ExecutorCache.java\nInit-on-demand holders]
JavaORC[GpuOrcTimezoneUtils.java\nfinal utility class]
JavaCC[CombineConf / AutoCloseableTargetSize\nJava value classes]
JavaTP[ThreadPoolConf interface\nDefaultThreadPoolConf / MemoryBoundedPoolConf]
JavaDB[DeviceBuffersUtils.java]
JavaHA[HostAllocResult.java]
end
ScalaEC -->|deleted / replaced by| JavaEC
ScalaORC -->|deleted / replaced by| JavaORC
ScalaCC -->|deleted / replaced by| JavaCC
GpuReaderFactory[GpuReaderFactory.scala\niceberg] -->|new CombineConf| JavaCC
WithRetrySuite[WithRetrySuite.scala\ntests] -->|new AutoCloseableTargetSize| JavaCC
Reviews (1): Last reviewed commit: "Move columnar runtime config helpers to ..." | Re-trigger Greptile |
| private static void closeAll(ColumnView[] views) { | ||
| for (ColumnView view : views) { | ||
| if (view != null) { | ||
| view.close(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The
closeAll(ColumnView[]) overload does not guard against exceptions thrown by view.close(). If element i throws, elements i+1 through the end of newColumns are never closed, leaking native GPU memory. The companion closeAll(List<ColumnView>) in the same file and DeviceBuffersUtils.closeAll both handle this correctly by catching each element's exception individually.
| private static void closeAll(ColumnView[] views) { | |
| for (ColumnView view : views) { | |
| if (view != null) { | |
| view.close(); | |
| } | |
| } | |
| } | |
| private static void closeAll(ColumnView[] views) { | |
| RuntimeException firstException = null; | |
| for (ColumnView view : views) { | |
| if (view != null) { | |
| try { | |
| view.close(); | |
| } catch (RuntimeException e) { | |
| if (firstException == null) { | |
| firstException = e; | |
| } else { | |
| firstException.addSuppressed(e); | |
| } | |
| } | |
| } | |
| } | |
| if (firstException != null) { | |
| throw firstException; | |
| } | |
| } |
Related to #14834.
Description
This PR is one reviewable layer in the unshim stack introduced by #15025. It moves columnar runtime configuration helpers into Java-friendly sources so runtime configuration access can be shared by later caller-migration layers.
Stack context
Testing and validation notes
Checklists
Documentation
Testing
(Covered by the validation notes in the PR description.)
Performance