[External] : Re: ClassHierarchyResolver using Reflection information
Brian Goetz
brian.goetz at oracle.com
Mon Mar 20 13:25:30 UTC 2023
If the user provides no resolver at all, what is the new behavior? Do
we just assume Object is the common supertype for any pair of classes?
On 3/20/2023 7:17 AM, Adam Sotona wrote:
>
> FYI: I’ve filled a new bug and proposed patch: *8304502: Classfile API
> class hierarchy makes assumptions when class is not resolved #13099
> <https://github.com/openjdk/jdk/pull/13099>*
>
> Thanks,
>
> Adam
>
> On 20.03.2023 10:50, "Adam Sotona" <adam.sotona at oracle.com> wrote:
>
> I agree with your concerns. The benevolence of the CHR was decision of
> an early development stage, it supposed to be temporary and now it is
> a clear bug.
>
> So from my perspective - yes, fail fast and fail when any required
> class is not resolved.
>
> Classfile API as well as ASM are designed to handle majority of cases
> without user knowledge of CHR or class loading details. I guess only
> few ASM users know how to pass custom class loader for correct CHR for
> stack maps generation. I’ll try to summarize benefits and negatives of
> my proposal to extend default CHR to fall back to a class loading CHR.
>
> Benefits of CHR by classfile parsing:
>
> -Performance – “skip-parsing” of constant pool + classfile header is
> in order of magnitude faster than class loading
>
> -Simplicity/Stability – it is not necessary to provide all
> dependencies to identify if the type is an interface or what is its
> super. It is not necessary to have complete classpath with
> implementation of some API to generate code using the API.
>
> Negatives of CHR by classfile parsing:
>
> -Availability - a classfile may not be available or accessible as a
> resource for parsing
>
> -Different results – a classfile may be modified when loaded, however
> the modification would have to change its super class or its status of
> being an interface to make any impact.
>
> By adding a fallback to class loading CHR we will eliminate the
> “Availability“ negative, with no performance loose.
>
> I think that the default should be rock-solid, because 99% of users
> will use the default.
>
> Thanks,
>
> Adam
>
> BTW: Maybe even more beneficial would be an ability to question
> already loaded classes first (without loading them), parse them as
> resource if not already loaded and fallback to load them if not
> available as resource.
>
> On 18.03.2023 5:06, "Michael van Acken" <michael.van.acken at gmail.com>
> wrote:
>
> Am Fr., 17. März 2023 um 19:34 Uhr schrieb Brian Goetz
> <brian.goetz at oracle.com>:
>
> I'm not so comfortable adding this to DEFAULT_CHA. In addition to
> adding another degree of environmental dependence, and the issues of
> additional exceptions you raise, I suspect it may also have a
> performance cost. I would like for programmers to opt into which CHA
> they use, and the default should be the most minimal. This means
> having
> a menu of CHAs to choose from, rather than guessing what the user
> wants.
>
> The default classHierarchyResolver has an opaque (to me) error
> mode that tripped me up. Similar to my experience with ASM some
> years ago, it works fine for a lot (most?) of code, but then some
> input causes it to silently produce a stack map the verifier does
> not agree with. For me, this happens usually quite late in the
> game and at first is completely baffling. Back in July it was
> mostly luck and half buried memories that sent me into the
> direction of CHR as the solution to the verifier error. If the
> default CHR had raised an exception at the point where
> information was missing, the path to the fix (preloading the CHR
> with information about future classes) would have been clearer.
>
> --mva
>
>
> On 3/17/2023 2:25 PM, - wrote:
> > I've submitted a patch at
> https://github.com/openjdk/jdk/pull/13082
> <https://urldefense.com/v3/__https:/github.com/openjdk/jdk/pull/13082__;!!ACWV5N9M2RV99hQ!IckIM3pDx0kj8-sYmKqh2mYHCcMDA3jEXIwOwzMAsirh8YKg1N949owV5wzMUHjA8_uF8Seb8GjSAMGhFN0zL4nTfIE$>
> > Yet with it comes 3 questions:
> >
> > 1. Should the resolver fail fast on IllegalAccessException from the
> > lookup? This usually indicates the hierarchy resolver is set up
> > improperly, and proceeding may simply yield verification errors in
> > class loading that are hard to track. For bytecode-generating APIs,
> > throwing access errors for the Lookup eagerly is also more
> preferable
> > than later silent generation failure.
> >
> > The main concern for me to fail fast is that I don't understand how
> > the Classfile API propagates resolver exceptions. If wrapping the
> > IllegalAccessException in an IllegalAccessError is safe, then I may
> > change it and add a test case with HashMap.
> >
> > 2. Whether the default resolver should be reading from jrt alone,
> > reflection alone, or jrt then reflection. I personally believe
> > reflection alone is more reliable, for classes may be redefined with
> > instrumentation or jfr, which may not be reflected in the system
> > resources.
> >
> > This idea came to me while I was working on jfr and instrumentation
> > tests. Luckily, it seems they don't touch class hierarchy that the
> > default resolver suffices.
> >
> > 3. In addition, I don't think chaining system class loader
> reflection
> > after system resource retrieval is really meaningful: is there any
> > case where reflection always works while the system resource
> retrieval
> > always fails?
> >
> > Chen
> >
> > On Fri, Mar 17, 2023 at 7:11 AM Adam Sotona
> <adam.sotona at oracle.com> wrote:
> >> I like your proposal and I would like to see it as a fallback
> solution for DEFAULT_CLASS_HIERARCHY_RESOLVER when the class is
> not available as resource stream.
> >>
> >>
> >>
> >> /**
> >>
> >> * Default singleton instance of {@linkplain
> ClassHierarchyResolver}
> >>
> >> * using {@link ClassLoader#getSystemResourceAsStream(String)}
> >>
> >> * as the {@code ClassStreamResolver} with fallback to
> >>
> >> * {@link ClassLoader.getSystemClassLoader()} class
> loading resolver.
> >>
> >> */
> >>
> >> ClassHierarchyResolver DEFAULT_CLASS_HIERARCHY_RESOLVER
> >>
> >> = new ClassHierarchyImpl.CachedClassHierarchyResolver(
> >>
> >> new Function<ClassDesc, InputStream>() {
> >>
> >> @Override
> >>
> >> public InputStream apply(ClassDesc classDesc) {
> >>
> >> return
> ClassLoader.getSystemResourceAsStream(Util.toInternalName(classDesc)
> + ".class");
> >>
> >> }
> >>
> >>
> }).orElse(ClassHierarchyResolver.of(ClassLoader.getSystemClassLoader()));
> >>
> >>
> >>
> >> Thanks,
> >>
> >> Adam
> >>
> >>
> >>
> >> On 16.03.2023 17:05, "liangchenblue at gmail.com"
> <liangchenblue at gmail.com> wrote:
> >>
> >>
> >>
> >> For context, in 8294961
> >> https://github.com/openjdk/jdk/pull/10991/files#r1133086265
> <https://urldefense.com/v3/__https:/github.com/openjdk/jdk/pull/10991/files*r1133086265__;Iw!!ACWV5N9M2RV99hQ!IckIM3pDx0kj8-sYmKqh2mYHCcMDA3jEXIwOwzMAsirh8YKg1N949owV5wzMUHjA8_uF8Seb8GjSAMGhFN0zkG4CkgE$>
> I wondered
> >> about whether to use a hierarchy resolver for
> LambdaMetafactory, that
> >> I think resolution may encounter problems as the default
> resolver may
> >> not be able to resolve user-supplied interfaces.
> >>
> >> In addition, many of the class file generation usages I've seen
> >> include firing events via an event bus, calling patched
> external addon
> >> code (as seen in Minecraft modding), in which Reflection-based
> >> hierarchy resolution would work better than reading bytecode files.
> >>
> >> Thus, I've prepared a patch creating ClassHierarchyResolver
> >>
> https://github.com/openjdk/jdk/commit/0c888ba1e2953cf946012244446f4f8c05ba5d77
> <https://urldefense.com/v3/__https:/github.com/openjdk/jdk/commit/0c888ba1e2953cf946012244446f4f8c05ba5d77__;!!ACWV5N9M2RV99hQ!IckIM3pDx0kj8-sYmKqh2mYHCcMDA3jEXIwOwzMAsirh8YKg1N949owV5wzMUHjA8_uF8Seb8GjSAMGhFN0zl_RRNVo$>
> >> to ease up resolution with reflection, when a ClassLoader (for
> older
> >> APIs) or a MethodHandle.Lookup (for modern APIs) is available.
> >>
> >> Does this appear feasible?
> >>
> >> Chen
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230320/29a10650/attachment-0001.htm>
More information about the classfile-api-dev
mailing list