[GLUTEN-9149][CORE] Remove Spark-specific code from JniLibLoader & JniWorkspace#9150
[GLUTEN-9149][CORE] Remove Spark-specific code from JniLibLoader & JniWorkspace#9150zzcclp merged 10 commits intoapache:mainfrom
Conversation
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten ClickHouse CI on ARM |
1 similar comment
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI on ARM |
1 similar comment
|
Run Gluten ClickHouse CI on ARM |
gluten-core/src/main/java/org/apache/gluten/jni/JniWorkspace.java
Outdated
Show resolved
Hide resolved
|
Run Gluten ClickHouse CI on ARM |
1 similar comment
|
Run Gluten ClickHouse CI on ARM |
| static { | ||
| SparkShutdownManagerUtil.addHookForLibUnloading( | ||
| () -> { | ||
| forceUnloadAll(); |
There was a problem hiding this comment.
It seems the forceUnloadAll is used to fix the coredump in ch backends. Can we remove this now? @taiyang-li @baibaichen
There was a problem hiding this comment.
Yes, it is still needed.
There was a problem hiding this comment.
It seems can not be removed, or will still coredump
philo-he
changed the title
philo-he
left a comment
philo-he
left a comment
There was a problem hiding this comment.
Thanks for your work! Two comments.
|
|
||
| public static JniWorkspace getDefault() { | ||
| public static void initializeDefault(String rootDir) { | ||
| synchronized (DEFAULT_INSTANCE_INIT_LOCK) { |
There was a problem hiding this comment.
Do we still need synchronized? Seems it can only be executed by one thread in initialize context after this refactoring.
There was a problem hiding this comment.
In tests, driver and executors are in the same process, so initialize may be call by driver and executor at the same time.
| } | ||
|
|
||
| public static JniWorkspace getDefault() { | ||
| synchronized (DEFAULT_INSTANCE_INIT_LOCK) { |
There was a problem hiding this comment.
@shuai-xu, seems no need to add "synchronized" in getting the instance.
|
@philo-he @jackylee-ch is it ok to merge this pr ? |
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxListenerApi.scala
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| public static JniWorkspace getDefault() { | ||
| synchronized (DEFAULT_INSTANCE_INIT_LOCK) { |
There was a problem hiding this comment.
@shuai-xu, seems no need to add "synchronized" in getting the instance.
|
Run Gluten ClickHouse CI on ARM |
philo-he
left a comment
philo-he
left a comment
There was a problem hiding this comment.
Looks good! One trivial comment only.
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxListenerApi.scala
Outdated
Show resolved
Hide resolved
|
Run Gluten ClickHouse CI on ARM |
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxListenerApi.scala
Outdated
Show resolved
Hide resolved
|
Run Gluten ClickHouse CI on ARM |
|
Run Gluten ClickHouse CI |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten ClickHouse CI on ARM |
6 participants
What changes were proposed in this pull request?
Make JniWorkspace not depend on spark.
(Fixes: #9149)
How was this patch tested?
This patch was tested by existing unit tests and integration tests.