Initial webrev with changes for JDK 9

Mandy Chung mandy.chung at oracle.com
Wed Mar 16 18:50:43 UTC 2016


Hi Peter,

Thanks for the review feedback.

> 
> NamedPackage p = packages.get(name);
> if (p instanceof Package) {
>    return (Package) p;
> }
> 
> return (Package)packages.compute((n, p) -> {
>  // define Package object if it is not yet defined or replace it if it is a NamedPackage
>  return (p instanceof Package) ? p : NamedPackage.toPackage(n, m);
> });
> 
> See, no private ClassLoader.toPackage(String name, NamedPackage p) needed.
> 

I like this suggestion that allows me to remove this private method.  I have this patch:
   http://cr.openjdk.java.net/~mchung/jigsaw/webrevs/jigsaw-m3/webrev-03-16/

One thing to note is that I don’t view definePackage returning Package object is performance critical.

java.lang.Package is legacy and from my scan of its usage, it's mainly used for getting the package name.  The versioning info is more for tracing/debugging use.  Package objects has not been defined for every packages.  Not to mention its assumption on class loader hierarchy and  packages are not splitted among JAR files.

In jake, Alex and I rewrote the java.lang.Package class spec.  Also fixed several design issue of Package.  I’d recommend any use of Class.getPackage().getName() be replaced with Class::getPackageName which is more performant if performance is important to them.  Package annotation (package-info) is the main useful info to be obtained from Package object. Otherwise I am not aware anything in Package class needed to be performant.

> 
> If you also care for constant lambda, this could be optimized even further, but for the price of more complex code:

Simple clean code as above is good for this case.

> On Mar 16, 2016, at 10:30 AM, Peter Levart <peter.levart at gmail.com> wrote:
> 
> In java.lang.ClassLoader:
> 
> ...the package-private definePackage(String name, Module m) is OK to use a single packages.compute(...) call performance-wise since it is pre-screened with packages.get() in public getDefinedPackage(String name) method. But there's also a package-private packages() method (a basis for public methods getPackages() and getDefinedPackages()) that constructs a Stream<Package> of defined Packages which unnecessarily calls definePackage() for each value of packages map:
> 
>    Stream<Package> packages() {
>        return packages.values().stream()
>                       .map(p -> definePackage(p.packageName(), p.module()));
>    }
> 
> 
> It would be nice performance-wise to avoid calling definePackage if the value is already a Package:
> 
>    Stream<Package> packages() {
>        return packages.values().stream()
>                       .map(p -> p instanceof Package
>                                 ? (Package) p
>                                 : definePackage(p.packageName(), p.module()));
>    }
> 

As explained above, getting a Package object is not performance critical. I’ll keep this in mind for other performance critical code.

thanks
Mandy



More information about the jigsaw-dev mailing list