<html><body><div style="font-family: arial, helvetica, sans-serif; font-size: 12pt; color: #000000"><div>For the caching part, it can be easier to ask each implementation to cache it's own results (like a ClassLoader does) given that depending on the implementation, the way to cache is quite different.<br></div><div><br data-mce-bogus="1"></div><div>regards,<br data-mce-bogus="1"></div><div>Rémi<br data-mce-bogus="1"></div><div><br></div><hr id="zwchr" data-marker="__DIVIDER__"><div data-marker="__HEADERS__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Brian Goetz" <brian.goetz@oracle.com><br><b>To: </b>"-" <liangchenblue@gmail.com>, "Adam Sotona" <adam.sotona@oracle.com><br><b>Cc: </b>"Michael van Acken" <michael.van.acken@gmail.com>, "classfile-api-dev" <classfile-api-dev@openjdk.org>, asotona@openjdk.org<br><b>Sent: </b>Monday, March 20, 2023 7:19:45 PM<br><b>Subject: </b>Re: [External] : Re: ClassHierarchyResolver using Reflection information<br></blockquote></div><div data-marker="__QUOTED_TEXT__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><font size="4"><font face="monospace">The CHR interface is
straightforward enough -- given a symbolic reference to a class,
provide two bits of information: superclass and is-interface. <br>
<br>
We have at least four sensible implementations, which can be
combined: <br>
- do nothing<br>
- work from an explicit static list (suitable for compilers who
are generating classes that do not yet exist)<br>
- Extract `.class` resources from the running JDK<br>
- Extract live Class objects from the running JDK<br>
<br>
These go in increasing order of work done and environmental
dependency; earlier items will be faster and more reproducible;
latter items will be more usable. The CHR interface also
supports chaining with `orElse`, so that we can start with one
strategy and fall back to another. <br>
<br>
Separately, there is caching, which should be orthogonal but
which is built into the default resolver. There's a public
CachedClassHierarchyResolver wrapper (probably should be
private) which delegates to another CHR and memoizes the result
in a synchronized HashMap. (There's a static factory ofCached
which delegates to this, but it is unused.)<br>
<br>
More importantly, the default adopts the cache strategy of
"cache lives forever / shared among all clients of the API." I
get that we want to avoid re-resolving these classes every time,
but if we're going to include class loading as a way to put
elements in the cache, there is a risk that it becomes polluted
with "private" stale data (and also retains memory forever.)<br>
<br>
So things I would like to fix include:<br>
<br>
- The basic implementations are not in a form that makes them
easily reusable;<br>
- It is not obvious exactly what the default resolver does (and
the name may be wrong), or when the user would want to select an
alternate strategy, or what its caching strategy is, but it
should definitely be defined in terms of building blocks the
user understands;<br>
- The caching strategy of the default is questionable and
probably needs more configuration.<br>
<br>
We currently define the default as:<br>
<br>
new CachedCHR( cd ->
ClassLoader.getSystemResourceAsStream(Util.toInternalName(classDesc)
+ ".class"))<br>
<br>
and it is being proposed that it be redefined as that, plus
falling back to class loading if that fails. <br>
<br>
What I'd like users to come away with is:<br>
<br>
- There's a menu of basic options, which can be combined;<br>
- There's a way to wrap the basic options with caching, and
ideally the user can have some control over sharing / lifetime /
caching strategy;<br>
- There's a default, which is defined strictly in terms of the
menu above, which they can reason about.<br>
<br>
We can start with exposing the basics. We already have
CHR.of(interfaces, supersMap). We need factories or constants
for "do nothing", "system resources from classloader X", "live
classes from classloader X", etc. <br>
<br>
We can refine caching so that CachedCHR is a private
implementation, and all public API is funneled through
ofCached. It is possible we will want to expose some more
control over the cache, such as wrapping the whole cache with a
SoftRef, or allowing the user to specify the backing map; this
is one more factory. <br>
<br>
I think we should rename DEFAULT_CHR to something like
GLOBAL_CHR to reflect the fact that it comes with a long-lived,
JVM-wide cache. GLOBAL_CHR can be specified to be
CachedCHR.of(systemResourcesCHR()) or
CachedCHR.of(systemResourcesChr().orElse(systemClassloaderCHR())).
<br>
<br>
We can _then_ define DEFAULT_CHR to point to GLOBAL_CHR, and
people who want a non-shared cache can substitute one. <br>
<br>
Smaller observation: we have shied away from using public
records in the API, instead having a public interface and a
private record-backed implementation. ClassHierarchyInfo is
currently a public record. <br>
<br>
<br>
Stepping back, the global cache suggests that we might want to
refactor the front-door entry point to the API. Right now, we
have a pile of static methods in Classfile (build/parse), many
of which take sets of options. The point of the global cache is
to avoid re-resolving information every time, but the global
granularity is questionable -- it seems much more likely that an
agent would want to create a static per-agent configuration,
shared across uses of that agent, but not shared with other
agents. This suggests that perhaps the methods on Classfile
really should be instance methods on an object that encapsulates
the option set as well as accumulating cache state. (And if the
agent is unloaded, all the cache goes with it.) <br>
<br>
Something like:<br>
<br>
public class ClassfileReaderWriterThingie { <br>
public ClassfileReaderWriterThingie(Option... options) {
... }<br>
<br>
</font></font><font size="4"><font face="monospace"><font size="4"><font face="monospace"> public ClassModel parse(byte[] bs)
{ ... }<br>
</font></font></font></font><font size="4"><font face="monospace"><font size="4"><font face="monospace"><font size="4"><font face="monospace"> public ClassModel
parse(Path p) { ... }<br>
<br>
public byte[] build(ClassDesc thisClass,
Consumer<CB> handler) { ... }<br>
// drop overload taking options, since its in
the ctor<br>
public byte[] build(ClassEntry thisClassEntry,
CPB cpBuilder, Consumer<CB> handler) { ... }<br>
<br>
// buildTo and buildModule overloads<br>
}<br>
</font></font></font></font><br>
This is a somewhat more intrusive API change (though not too
much), so my suggestion is to proceed on the above first and
then revisit this one. <br>
<br>
(One point being that if we're going to have a long-lived cache,
it should probably be encapsulated in an object the user can
control.)<br>
<br>
<br>
</font></font>
<div class="moz-cite-prefix">On 3/20/2023 12:51 PM, Brian Goetz
wrote:<br>
</div>
<blockquote cite="mid:17d29737-43f9-2713-da67-459ab59f1cc8@oracle.com">
<font size="4"><font face="monospace">After looking over this
corner of the API, I think there's a little more design work
that needs to go into it. So let's step back from the details
of "what's the default" until we have a chance to look a
little more holistically at this part of the API. I'll try to
write more about it soon. </font></font><br>
<br>
<div class="moz-cite-prefix">On 3/20/2023 11:47 AM, - wrote:<br>
</div>
<blockquote cite="mid:CABe8uE3CRsZ=+GTHamo0OSH5KAqkALP=yDBREhtNYwhwQPZ2vA@mail.gmail.com">
<pre class="moz-quote-pre">If the choice of default resolver is still subject to debate, I can
remove the delegation to system class loader based resolver in
<a class="moz-txt-link-freetext" href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/13082__;!!ACWV5N9M2RV99hQ!K7UWlrg2NHotSKOzMvmfnenKZ1dDuOZ9zXcQvlKOdB21VvwTRIDuI2NyRNK6Hq844XIkt_6Q-qFt5DsFDUzgKJ31$" target="_blank">https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/13082__;!!ACWV5N9M2RV99hQ!K7UWlrg2NHotSKOzMvmfnenKZ1dDuOZ9zXcQvlKOdB21VvwTRIDuI2NyRNK6Hq844XIkt_6Q-qFt5DsFDUzgKJ31$</a> , and we can change it in
another patch once we reach a consensus. The addition of lookup and
class loader based resolvers can be used by users who explicitly
specify a resolver in the options.
On Mon, Mar 20, 2023 at 9:00 AM Adam Sotona <a class="moz-txt-link-rfc2396E" href="mailto:adam.sotona@oracle.com" target="_blank"><adam.sotona@oracle.com></a> wrote:
</pre>
<blockquote>
<pre class="moz-quote-pre">New behavior is “no assumptions” and throw IllegalArgumentException if information about type necessary to build stack map (or verify stack map) is not obtained from the resolver (or better say chain of resolvers).
If user provides no resolver – the default is used.
The way how default resolver gets the information from system classloader is subject of discussion below (resource parsing and fallback to class loading).
Custom resolver can be constructed as a fallback chain from all provided resolver types (including the default resolver instance).
On 20.03.2023 14:25, "Brian Goetz" <a class="moz-txt-link-rfc2396E" href="mailto:brian.goetz@oracle.com" target="_blank"><brian.goetz@oracle.com></a> wrote:
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?
</pre>
</blockquote>
</blockquote>
<br>
</blockquote>
<br><br></blockquote></div></div></body></html>