RFR: Initial code for a JSON-based module descriptor.

[Shi Jun Zhang]

On 4/19/2012 8:05 PM, David Bosschaert wrote:
> Following the discussion about module descriptor formats on jigsaw-dev
> and penrose-dev [1] I did an initial implementation that supports a
> JSON-based module descriptor.
>
> The JSON module descriptor is located inside the module in
> META-INF/module-info.json.
> The existing module-info.class file is still supported but if a
> module-info.json file is found this is used instead.
> I wrote a very small JSON parser, its class files are currently<
> 10kb, and it can possibly be made even smaller.
> Besides the unit tests _JSONParser and _ModuleInfoReader there is also
> a system-level test hello-json.sh that effectively compiles, installs
> and runs a small demo module with JSON metadata.
> For example module-info.json files see inside the hello-json.sh and
> module-info-reader.sh test scripts.
>
> Some other things to note:
> * Not all features of module-info.class are yet supported, this is
> just a starting point.
> * The fact that both module-info.class and module-info.json are
> supported side-by-side makes for some awkward code here and there.
> This should only be temporary.
> * The JSON parser is currently in the java.lang.module package, but
> it's pretty much general-purpose, so I guess it could be moved to a
> package like java.util or something like that.
>
> You can find the webrev here:
> http://cr.openjdk.java.net/~davidb/mijson/webrev.01/
>
> David
>
> [1] http://mail.openjdk.java.net/pipermail/penrose-dev/2012-April/000038.html
>
Hi David,

I get the following exception when i run the test you added.

chance at chance:~/workspace/penrose/jigsaw/jdk/test/org/openjdk/jigsaw/z.test$ 
../../../../../build/bin/java -L z.mlib -m com.greetings.json
Error occurred during initialization of VM
java.lang.InternalError: Recursive initialization of system class loader
     at java.lang.ClassLoader.initSystemClassLoader(ClassLoader.java:1515)
     at java.lang.ClassLoader.getSystemClassLoader(ClassLoader.java:1494)
     at java.util.ServiceLoader.loadInstalled(ServiceLoader.java:618)
     at 
sun.util.LocaleServiceProviderPool$1.run(LocaleServiceProviderPool.java:131)
     at java.security.AccessController.doPrivileged(Native Method)
     at 
sun.util.LocaleServiceProviderPool.<init>(LocaleServiceProviderPool.java:129)
     at 
sun.util.LocaleServiceProviderPool.getPool(LocaleServiceProviderPool.java:111)
     at java.text.NumberFormat.getInstance(NumberFormat.java:747)
     at java.text.NumberFormat.getNumberInstance(NumberFormat.java:407)
     at java.util.Scanner.useLocale(Scanner.java:1231)
     at java.util.Scanner.<init>(Scanner.java:585)
     at java.util.Scanner.<init>(Scanner.java:608)
     at java.lang.module.JSONParser.<init>(JSONParser.java:87)
     at 
java.lang.module.JSONModuleInfoReader.<init>(JSONModuleInfoReader.java:61)
     at 
java.lang.module.JSONModuleInfoReader.read(JSONModuleInfoReader.java:47)
     at 
java.lang.module.ModuleSystem.parseModuleInfoJSON(ModuleSystem.java:80)
     at 
org.openjdk.jigsaw.SimpleLibrary.findModuleDir(SimpleLibrary.java:763)
     at 
org.openjdk.jigsaw.SimpleLibrary.readLocalModuleInfoJSON(SimpleLibrary.java:855)
     at org.openjdk.jigsaw.Library.readLocalModuleInfo(Library.java:142)
     at org.openjdk.jigsaw.Catalog.readModuleView(Catalog.java:277)
     at org.openjdk.jigsaw.Launcher.loadModule(Launcher.java:51)
     at org.openjdk.jigsaw.Launcher.launch(Launcher.java:82)
     at 
java.lang.ClassLoader.initModularSystemClassLoader(ClassLoader.java:1557)
     at java.lang.ClassLoader.initSystemClassLoader(ClassLoader.java:1522)
     at java.lang.ClassLoader.getSystemClassLoader(ClassLoader.java:1494)

Can you verify whether you are using the latest code? As you just read 
the whole json file into JSONParser, it's not necessary using the Scanner.

And some comments on the changes.

src/share/classes/java/lang/module/ModuleClassLoader.java

   72         ModuleInfo mi;
   73
   74         // Weird: getResource/getResources() return matching files from all modules, even if not exported.
   75         // URL miXML = getResource("/META-INF/module-info.xml");
   76         URL miJSON = null;
   77         try {
   78             for (Enumeration<URL>  infos = getResources("/META-INF/module-info.json"); infos.hasMoreElements(); ) {
   79                 URL info = infos.nextElement();
   80                 String suffix = id.name() + "/" + id.version().toString() + "/classes!/META-INF/module-info.json";
   81                 if (info.toExternalForm().endsWith(suffix)) {
   82                     miJSON = info;
   83                     break;
   84                 }
   85             }
   86         } catch (IOException e) {
   87             throw new Error(e);
   88         }
   89
   90         if (miJSON != null) {
   91             try {
   92                 mi = moduleSystem.parseModuleInfoJSON(miJSON.openStream());
   93             } catch (IOException e) {
   94                 throw new Error(e);
   95             }
   96         } else {
   97             mi = moduleSystem.parseModuleInfo(bs);
   98         }

ModuleClassLoader doesn't implement its own getResource() method so the 
getResource invocation will call java.lang.ClassLoader.getResource and 
it will delegate to parent's class loader first. I guess using 
((org.openjdk.jigsaw.Loader)this).getResource("/META-INF/module-info.json") 
will work. Although another method isModulePresent in ModuleClassLoader 
also cast itself to org.openjdk.jigsaw.Loader, i don't think casting to 
a subclass is a good design. A better way to solve this problem is that 
not changing the defineModule method but adding a new method named 
defineModuleByJSON which accepts JSON input stream. Then in 
org.openjdk.jigsaw.Loader.findModule method, call defineModuleByJSON 
method if module-info.json exists, else call defineModule method.

src/share/classes/java/lang/module/JSONParser.java
Line 152: jsonKeyValueList.charAt(i) is used for multiple times, assign 
it to a local var.
The parser is pretty small but i am worried about the efficiency. The 
more layers the JSON string has, more times the end part of the JSON 
string will be parsed. For example,
{
     "key1" :
     {
         "key2" :
         {
             "key3" : "value"
         }
     }
}
First time, it go through the whole string and find one key-value string 
and then use regex to match "key1" and its value string. Second time, it 
go through the string '"key2": { "key3" : "value" }' and find one 
key-value string and then use regex to match "key2" and its value 
string. Third time, it go through the string '"key3" : "value"' and find 
one key-value string and then use regex to match "key3" and its value 
"value". The performance degrades fast when the JSON structure becomes 
complex.

Other changes look ok to me.

-- 
Regards,

Shi Jun Zhang