[External] : Re: ClassHierarchyResolver using Reflection information

[Brian Goetz]

After the below is integrated, I'd like to propose the following API 
changes.

When I reviewed the code for cached hierarchy resolvers, I was concerned 
that the cache is hidden and shared across all users, meaning it will 
continue to fill up over time.  The change outlined below provide half 
the story, which is allowing more control over the locus of caching.  
The other half is making that control map easily to abstractions that 
users deal with.

What I'd like to do is move the static methods in Classfile onto some 
sort of ClassfileReaderWriter object (needs a better name.)  Then, the 
CRW becomes a sensible locus of caching, as well as a place to hang 
options.  (Yet again, something that seemingly made sense to be a static 
method at the beginning, has gotten complicated enough that it bites us.)

interface ClassfileReaderWriter {
     static CRW of() { ... }
     static CRW of(Collection<Option> options) { ... }

     byte[] build(ClassDesc thisClass);
     byte[] build(ClassDesc thisClass, Consumer<? super ClassBuilder> 
handler);
     byte[] build(ClassEntry thisClassEntry,
                  ConstantPoolBuilder constantPool,
                  Consumer<? super ClassBuilder> handler);

     byte[] buildModule(ModuleAttribute moduleAttribute);
     byte[] buildModule(ModuleAttribute moduleAttribute,
                        Consumer<? super ClassBuilder> handler);

     // similar buildTo and buildModuleTo overloads

     ClassModel parse(byte[] bytes);
     ClassModel parse(Path path);
}

The CRW::of factories act as caching points for the CHR and for the 
options.

Additionally, I think we can simplify the options as well with an ADT.  
Right now, we have an internal set of keys, an external set of factories 
that match up with the keys, an internal option carrier, and internal 
consumers have to blind-cast -- and we limit ourselves to options that 
have one parameter.

Instead, we can use a transparent ADT:

     sealed interface Option { }
     record GenerateStackMaps(boolean generateMaps) implements Option { }
     record ClassHierarchyResolver(ClassHierarchyResolver resolver) 
implements Option { }
     ...

and eliminate the factories, key enum, and internal cruft.

Further, breaking the limitation of "exactly one parameter" means we can 
refactor the options records to group related options if desired.

On 3/22/2023 2:55 PM, Adam Sotona wrote:
>
> Based on the discussion in this thread I propose to make a first step 
> and adjust ClassHierarchy factory methods following way:
>
>   * ofCached(ClassHierarchyResolver resolver)
>   * ofClassLoading(ClassLoader loader)
>   * ofResourceParsing(ClassLoader loader)
>   * ofParsing(Function<ClassDesc, InputStream> classStreamResolver)
>   * of(MethodHandles.Lookup lookup)
>   * of(Collection<ClassDesc> interfaces, Map<ClassDesc, ClassDesc>
>     classToSuperClass)
>
> So the default can be defined as:
>
> DEFAULT_CLASS_HIERARCHY_RESOLVER = 
> ofCached(ofResourceParsing(ClassLoader.getSystemClassLoader()).orElse(ofClassLoading(ClassLoader.getSystemClassLoader())));
>
> Please help me to polish exact methods naming.
>
> Thank you,
>
> Adam
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230419/66a34908/attachment-0001.htm>