<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<font size="4"><font face="monospace">I don't disagree; my primary
concern is that currently we have a default whose behavior is
fairly surprising with respect to cross-user interference and
long-term resource utilization. <br>
<br>
(Encapsulating the cache inside a ClassfileReaderWriterThingie
goes a long way towards mitigating that.)<br>
</font></font><br>
<div class="moz-cite-prefix">On 3/20/2023 2:29 PM, Remi Forax wrote:<br>
</div>
<blockquote type="cite" cite="mid:1173644065.15928935.1679336945185.JavaMail.zimbra@u-pem.fr">
<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" <a class="moz-txt-link-rfc2396E" href="mailto:brian.goetz@oracle.com"><brian.goetz@oracle.com></a><br>
<b>To: </b>"-" <a class="moz-txt-link-rfc2396E" href="mailto:liangchenblue@gmail.com"><liangchenblue@gmail.com></a>, "Adam
Sotona" <a class="moz-txt-link-rfc2396E" href="mailto:adam.sotona@oracle.com"><adam.sotona@oracle.com></a><br>
<b>Cc: </b>"Michael van Acken"
<a class="moz-txt-link-rfc2396E" href="mailto:michael.van.acken@gmail.com"><michael.van.acken@gmail.com></a>, "classfile-api-dev"
<a class="moz-txt-link-rfc2396E" href="mailto:classfile-api-dev@openjdk.org"><classfile-api-dev@openjdk.org></a>, <a class="moz-txt-link-abbreviated" href="mailto:asotona@openjdk.org">asotona@openjdk.org</a><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" moz-do-not-send="true">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" moz-do-not-send="true"><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" moz-do-not-send="true"><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>
</blockquote>
<br>
</body>
</html>