Use ServiceLoader to load RegExp and XML implementations #1799
Conversation
gbrail
commented
Jan 12, 2025
gbrail
commented
- Use ServiceLoader to load RegExp and XML implementations
- Don't forget new files
This is a more modern way than just classloading and could allow for some more flexibility in the future.
|
This PR uses the ServiceLoader to load the implementations of Regexp and XML objects, as opposed to what we do now, which is to load classes based on hard-coded class names. It also moves initialization of the specific objects required by XML to the XML implementation and gets that code out of ScriptRuntime. I think that this is a bit cleaner than we what did before, and it theoretically lets us replace these implementations in future versions of Rhino by just changing the classpath. On the other hand, in both cases we just take the first service that we find, which is not how ServiceLoader is normally used, since if there is more than one implementation we run the risk of inconsistency. I'm open to suggestions as to other ways to do this, or whether it's worth it at all, but I like the way that this gets the XML stuff out of the main codebase at least. This is all leading up to another PR where I'm going to try and use a simpler "lazy loading" mechanism for most of the standard objects to try and keep the loading of the standard library objects as fast as we can. |
| XMLLoader loader = loadOneServiceImplementation(XMLLoader.class); | ||
| if (loader != null) { | ||
| loader.load(scope, sealed); | ||
| } |
There was a problem hiding this comment.
CHECKME: Should we log some warning in future, if FEATURE_E4X is enabled (unfortunately, it is by default), but no XMLLoader found?
| public XMLLib.Factory getE4xImplementationFactory() { | ||
| return getFactory().getE4xImplementationFactory(); | ||
| XMLLoader loader = ScriptRuntime.loadOneServiceImplementation(XMLLoader.class); |
There was a problem hiding this comment.
INFO: As far as I know, this will create a new instance of XMLLoader every time.
