mozilla / rhino

Rhino is an open-source implementation of JavaScript written entirely in Java
https://rhino.github.io
Other
4.18k stars 851 forks source link

Draft for reading configuration from config files #1722

Open rPraml opened 2 weeks ago

rPraml commented 2 weeks ago

This could be a first draft for #1720

gbrail commented 1 week ago

I think that this approach makes sense in that it lets configuration come from the environment, system properties, file, or classpath. I think that's a good thing and that we should follow this pattern.

For actually checking the setting of various flags, however, I think that the config class should parse all this input and set boolean (or enum) values that can be checked directly, rather than relying on a hash lookup. I think that can give us two advantages:

1) It keeps all the logic about naming and renaming things in one place 2) It will be faster, since many of these properties, like the various debug flags, will be checked many millions of times potentially while Rhino runs, and all those hash table lookups will dominate performance pretty soon.

rPraml commented 2 days ago

I think I can implement this next week or so...

gbrail commented 1 day ago

If you're looking for the kinds of debug flags I'd like to potentially replace with a real debugging mechanism, I would look at these:

https://github.com/mozilla/rhino/blob/3e899f6420dc6260380d0a688016207f64f9cfec/rhino/src/main/java/org/mozilla/javascript/Token.java#L27

https://github.com/mozilla/rhino/blob/3e899f6420dc6260380d0a688016207f64f9cfec/rhino/src/main/java/org/mozilla/classfile/ClassFileWriter.java#L4476

https://github.com/mozilla/rhino/blob/3e899f6420dc6260380d0a688016207f64f9cfec/rhino/src/main/java/org/mozilla/javascript/optimizer/DefaultLinker.java#L25

And as for feature flags, the first one IMO should be for the reflect and proxy support that's currently languishing in a PR by @rbri