This is an automated email from the ASF dual-hosted git repository.
hongze pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-gluten.git
The following commit(s) were added to refs/heads/main by this push:
new de140c6011 [GLUTEN-8949][Core] Simplify synchronisation from
JniLibLoader (#8950)
de140c6011 is described below
commit de140c6011a8cd5e9dc5aaee791e5062e8e96bf3
Author: Arnav Balyan <[email protected]>
AuthorDate: Tue Mar 18 14:31:06 2025 +0530
[GLUTEN-8949][Core] Simplify synchronisation from JniLibLoader (#8950)
Closes #8949
---
.../java/org/apache/gluten/jni/JniLibLoader.java | 131 +++++++++------------
1 file changed, 57 insertions(+), 74 deletions(-)
diff --git a/gluten-core/src/main/java/org/apache/gluten/jni/JniLibLoader.java
b/gluten-core/src/main/java/org/apache/gluten/jni/JniLibLoader.java
index 384fb77528..2d07c3d79b 100644
--- a/gluten-core/src/main/java/org/apache/gluten/jni/JniLibLoader.java
+++ b/gluten-core/src/main/java/org/apache/gluten/jni/JniLibLoader.java
@@ -45,10 +45,8 @@ import scala.runtime.BoxedUnit;
public class JniLibLoader {
private static final Logger LOG =
LoggerFactory.getLogger(JniLibLoader.class);
- private static final Set<String> LOADED_LIBRARY_PATHS =
- Collections.synchronizedSet(new HashSet<>());
- private static final Set<String> REQUIRE_UNLOAD_LIBRARY_PATHS =
- Collections.synchronizedSet(new LinkedHashSet<>());
+ private static final Set<String> LOADED_LIBRARY_PATHS = new HashSet<>();
+ private static final Set<String> REQUIRE_UNLOAD_LIBRARY_PATHS = new
LinkedHashSet<>();
static {
SparkShutdownManagerUtil.addHookForLibUnloading(
@@ -59,17 +57,15 @@ public class JniLibLoader {
}
private final String workDir;
- private final Set<String> loadedLibraries = Collections.synchronizedSet(new
HashSet<>());
+ private final Set<String> loadedLibraries = new HashSet<>();
JniLibLoader(String workDir) {
this.workDir = workDir;
}
- public static void forceUnloadAll() {
+ public static synchronized void forceUnloadAll() {
List<String> loaded;
- synchronized (REQUIRE_UNLOAD_LIBRARY_PATHS) {
- loaded = new ArrayList<>(REQUIRE_UNLOAD_LIBRARY_PATHS);
- }
+ loaded = new ArrayList<>(REQUIRE_UNLOAD_LIBRARY_PATHS);
Collections.reverse(loaded); // use reversed order to unload
loaded.forEach(JniLibLoader::unloadFromPath);
}
@@ -89,19 +85,15 @@ public class JniLibLoader {
private static void loadFromPath0(String libPath, boolean requireUnload) {
libPath = toRealPath(libPath);
- synchronized (LOADED_LIBRARY_PATHS) {
- if (LOADED_LIBRARY_PATHS.contains(libPath)) {
- LOG.debug("Library in path {} has already been loaded, skipping",
libPath);
- } else {
- System.load(libPath);
- LOADED_LIBRARY_PATHS.add(libPath);
- LOG.info("Library {} has been loaded using path-loading method",
libPath);
- }
+ if (LOADED_LIBRARY_PATHS.contains(libPath)) {
+ LOG.debug("Library in path {} has already been loaded, skipping",
libPath);
+ } else {
+ System.load(libPath);
+ LOADED_LIBRARY_PATHS.add(libPath);
+ LOG.info("Library {} has been loaded using path-loading method",
libPath);
}
if (requireUnload) {
- synchronized (REQUIRE_UNLOAD_LIBRARY_PATHS) {
- REQUIRE_UNLOAD_LIBRARY_PATHS.add(libPath);
- }
+ REQUIRE_UNLOAD_LIBRARY_PATHS.add(libPath);
}
}
@@ -113,42 +105,36 @@ public class JniLibLoader {
loadFromPath0(file.getAbsolutePath(), requireUnload);
}
- public static void unloadFromPath(String libPath) {
- synchronized (LOADED_LIBRARY_PATHS) {
- if (!LOADED_LIBRARY_PATHS.remove(libPath)) {
- LOG.warn("Library {} was not loaded or already unloaded:", libPath);
- return;
- }
+ public static synchronized void unloadFromPath(String libPath) {
+ if (!LOADED_LIBRARY_PATHS.remove(libPath)) {
+ LOG.warn("Library {} was not loaded or already unloaded:", libPath);
+ return;
}
LOG.info("Starting unload library path: {} ", libPath);
- synchronized (REQUIRE_UNLOAD_LIBRARY_PATHS) {
- REQUIRE_UNLOAD_LIBRARY_PATHS.remove(libPath);
- }
+ REQUIRE_UNLOAD_LIBRARY_PATHS.remove(libPath);
try {
ClassLoader classLoader = JniLibLoader.class.getClassLoader();
Field field = ClassLoader.class.getDeclaredField("nativeLibraries");
field.setAccessible(true);
Vector<Object> libs = (Vector<Object>) field.get(classLoader);
- synchronized (libs) {
- Iterator<Object> it = libs.iterator();
- while (it.hasNext()) {
- Object object = it.next();
- Field[] fs = object.getClass().getDeclaredFields();
- for (int k = 0; k < fs.length; k++) {
- if (fs[k].getName().equals("name")) {
- fs[k].setAccessible(true);
- String verbosePath = fs[k].get(object).toString();
- File verboseFile = new File(verbosePath);
- String verboseFileName = verboseFile.getName();
- File libFile = new File(libPath);
- String libFileName = libFile.getName();
-
- if (verboseFileName.equals(libFileName)) {
- LOG.info("Finalizing library file: {}", libFileName);
- Method finalize =
object.getClass().getDeclaredMethod("finalize");
- finalize.setAccessible(true);
- finalize.invoke(object);
- }
+ Iterator<Object> it = libs.iterator();
+ while (it.hasNext()) {
+ Object object = it.next();
+ Field[] fs = object.getClass().getDeclaredFields();
+ for (int k = 0; k < fs.length; k++) {
+ if (fs[k].getName().equals("name")) {
+ fs[k].setAccessible(true);
+ String verbosePath = fs[k].get(object).toString();
+ File verboseFile = new File(verbosePath);
+ String verboseFileName = verboseFile.getName();
+ File libFile = new File(libPath);
+ String libFileName = libFile.getName();
+
+ if (verboseFileName.equals(libFileName)) {
+ LOG.info("Finalizing library file: {}", libFileName);
+ Method finalize =
object.getClass().getDeclaredMethod("finalize");
+ finalize.setAccessible(true);
+ finalize.invoke(object);
}
}
}
@@ -158,36 +144,33 @@ public class JniLibLoader {
}
}
- public void load(String libPath, boolean requireUnload) {
- synchronized (loadedLibraries) {
- try {
- if (loadedLibraries.contains(libPath)) {
- LOG.debug("Library {} has already been loaded, skipping", libPath);
- return;
- }
- File file = moveToWorkDir(workDir, libPath);
- loadWithLink(file.getAbsolutePath(), null, requireUnload);
- loadedLibraries.add(libPath);
- LOG.info("Successfully loaded library {}", libPath);
- } catch (IOException e) {
- throw new GlutenException(e);
+ public synchronized void load(String libPath, boolean requireUnload) {
+ try {
+ if (loadedLibraries.contains(libPath)) {
+ LOG.debug("Library {} has already been loaded, skipping", libPath);
+ return;
}
+ File file = moveToWorkDir(workDir, libPath);
+ loadWithLink(file.getAbsolutePath(), null, requireUnload);
+ loadedLibraries.add(libPath);
+ LOG.info("Successfully loaded library {}", libPath);
+ } catch (IOException e) {
+ throw new GlutenException(e);
}
}
- public void loadAndCreateLink(String libPath, String linkName, boolean
requireUnload) {
- synchronized (loadedLibraries) {
- try {
- if (loadedLibraries.contains(libPath)) {
- LOG.debug("Library {} has already been loaded, skipping", libPath);
- }
- File file = moveToWorkDir(workDir, libPath);
- loadWithLink(file.getAbsolutePath(), linkName, requireUnload);
- loadedLibraries.add(libPath);
- LOG.info("Successfully loaded library {}", libPath);
- } catch (IOException e) {
- throw new GlutenException(e);
+ public synchronized void loadAndCreateLink(
+ String libPath, String linkName, boolean requireUnload) {
+ try {
+ if (loadedLibraries.contains(libPath)) {
+ LOG.debug("Library {} has already been loaded, skipping", libPath);
}
+ File file = moveToWorkDir(workDir, libPath);
+ loadWithLink(file.getAbsolutePath(), linkName, requireUnload);
+ loadedLibraries.add(libPath);
+ LOG.info("Successfully loaded library {}", libPath);
+ } catch (IOException e) {
+ throw new GlutenException(e);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
