RFR: JDK-8222373 Improve CDS performance for custom class loaders

[yumin qi]

Hi, Alan and Ioi
  Thanks. Forget to add core-libs-dev for the review.
  If add a public API, surely it should be discussed in detail the design,
implementation and effects. One question, will adding a public API for
ClassLoader need go some process like CSR?
  I did not add public API here and tried to avoid such process, that may
not be a good choice for this case.

Thanks
Yumin

On Thu, Jun 20, 2019 at 12:12 AM Ioi Lam <ioi.lam at oracle.com> wrote:

>
>
> On 6/19/19 11:36 PM, Alan Bateman wrote:
> > On 20/06/2019 02:36, yumin qi wrote:
> >> Hi, Please review:
> >> bug: https://bugs.openjdk.java.net/browse/JDK-8222373
> >> webrev: http://cr.openjdk.java.net/~minqi/8222373/01/
> >>
> >> To load shared class from CDS, first class file stream is read from jar
> >> file, then load the class with the stream. In vm, the stream is used to
> >> calculate file size and crc32 which are used to compare with the counter
> >> parts stored in CDS.
> >>
> >> In fact, when CDS mapped, every SharedClassPathEntry is checked for
> >> validation, if there is mismatch JVM will exit. That is, if user updated
> >> jars and did not recreated CDS archive, it would fail. This can make us
> >> load the class from CDS directly (if it is in CDS) without checking
> >> class
> >> file length and crc32, so skip getting byte stream from source to
> >> save time.
> >>
> > cc'ing core-libs-dev as this proposal involves changes to the
> > ClassLoader API that will require significant discussion. One initial
> > concern is that it exposes the notion of "shared class" in the
> > standard API. I'm also concerned about the reliance on the protection
> > domain and changing existing defineClass methods to allow the class
> > bytes be null. How does that work when CDS is disabled - are you
> > expecting class loader implementation with go-faster stripes to retry
> > a different defineClass with the class bytes? Ioi has had a number of
> > proposals in this area (and I see he's added a comment to the bug) but
> > we didn't converge on the right API so maybe it time to have another
> > attempt at that issue first.
> >
> > -Alan
> I have a rough idea -- let's have a higher-level representation of the
> bytecode stream than byte[] or ByteBuffer, to make optimization possible.
>
> So we could have a new API in ClassLoader
>
>      protected final Class<?> defineClass(String name, String location,
> ProtectionDomain protectionDomain)
>
> and its behavior is equivalent to the following
>
>     {
>          byte[] b = read_buffer_from(location);
>          return defineClass(name, b, 0, b.len, protectionDomain);
>     }
>
>
> examples would be:
>
>       defineClass("java/lang/Object",
> "jrt:/java.base/java/lang/Object.class", NULL);
>       defineClass("com/foo/Bar",
> "file://path/com.foo.jar!com/foo/Bar.class", myProtectionDomain);
>
> Note that the type and value of <location> is just for illustrative
> purposes. We might use a different type (URI??). It's just a way to name
> a location that you can read a byte buffer from.
>
> The protectionDomain will need to be created by the caller. The use of
> the protectionDomain will be no different than the existing
> defineClass() APIs. Specifically, it will not be used in any way to
> fetch the buffer.
>
> When CDS is enabled, the VM can check if the name+location matches a
> class in the CDS archive. If so, the class is loaded without fetching
> the buffer.
>
> The caller doesn't need to know if CDS is enabled or not.
>
>
> (We probably don't want a String type but a more abstract type. That way
> we can evolve it to allow more complex representations, such as "read
> the bytecode stream from here, but replace the method name "Foo" to
> "Bar", and add a new integer field "X" ....
>
> If John Rose was to design this, I am sure he will call it something
> like BytecodeStreamDynamic :-)
>
> This may actually reduce the use of ASM. E.g., today people are forced
> to write their own bytecodes, even if they just want some simple
> transformation on template classes).
>
>
> Thanks
> - Ioi
>
>
>
>
>
>
>