LuciferYang opened a new issue, #12394:
URL: https://github.com/apache/gluten/issues/12394

   ### Describe the bug
   
   After #12393 lands, `JniLibLoader.load(libPath)` and 
`JniLibLoader.loadAndCreateLink(libPath, null)` are behaviourally identical:
   
   ```java
   public synchronized void load(String libPath) {
     try {
       if (loadedLibraries.contains(libPath)) {
         LOG.debug("Library {} has already been loaded, skipping", libPath);
         return;
       }
       File file = moveToWorkDir(workDir, libPath);
       loadWithLink(file.getAbsolutePath(), null);
       loadedLibraries.add(libPath);
       LOG.info("Successfully loaded library {}", libPath);
     } catch (IOException e) {
       throw new GlutenException(e);
     }
   }
   
   public synchronized void loadAndCreateLink(String libPath, String linkName) {
     try {
       if (loadedLibraries.contains(libPath)) {
         LOG.debug("Library {} has already been loaded, skipping", libPath);
         return;
       }
       File file = moveToWorkDir(workDir, libPath);
       loadWithLink(file.getAbsolutePath(), linkName);
       loadedLibraries.add(libPath);
       LOG.info("Successfully loaded library {}", libPath);
     } catch (IOException e) {
       throw new GlutenException(e);
     }
   }
   ```
   
   The only difference is the second argument to `loadWithLink`, and 
`loadWithLink` itself short-circuits the symlink branch when `linkName == 
null`. So one of the two methods is dead duplication once #12393 is merged.
   
   The duplication was previously masked by the missing-`return` bug fixed in 
#12393, which made `loadAndCreateLink` behave differently from `load` on a 
repeat call. With the bug fixed, the two diverge in exactly one place: the 
`linkName` argument.
   
   ### Expected behavior
   
   Replace the body of `load` with a delegate:
   
   ```java
   public synchronized void load(String libPath) {
     loadAndCreateLink(libPath, null);
   }
   ```
   
   One copy of the try/catch + dedup-check + log lines, one place to evolve if 
the loading sequence ever changes.
   
   Depends on #12393 being merged first.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to