Planned features of a classfile context object

[Brian Goetz]

> I like the idea when the whole “context” is out of the ClassModel, so 
> using it with a different context will do the job differently. However 
> doesn’t it mean they are rather mutable, when you get different 
> content based on context?
>

It means certain operations are, well, contextual.  If you ask a context 
to generate a class, it will do so with the options of that context.  So 
if you generate the same class twice with different contexts, you get 
different results.

It also means some entities might drag parts of their creating context 
with them.  For example, suppose we parse a classfile using context A, 
which has a mapper for attribute "Foo".  Then we adapt it using context 
B, that doens't know about Foo.  Well, B may not know about Foo, but the 
FooAttribute will have a reference to its creating AttributeMapper.  
That's fine too (well, if different contexts have radically different 
ideas of how a FooAttribute is laid out, maybe not, but in that case we 
have bigger problems.)


> Some of the options already behave that way and are quite flexible 
> (processDebug, generateStackmap, constantPoolSharing, fixShortJumps, 
> patchDeadCode, classHierarchyResolver, filterDeadLabels) and some can 
> be fixed to do so (processLineNumbers).
>

I think its been a while since we took a good look at our options and 
asked "is this the way we want it"; now is a good time to review this.

> However for example attributeMapper is closely tied with the parsed 
> model and it would require to re-parse when. Also the last used 
> context would have to be stored with the model to know if any reset of 
> already expanded model is needed (for example did we skipped parsing 
> line numbers or are they just missing?)
>

I think this is a place where we have to lean on existing design 
constraints of backward compatibility.  We can parse the attribute with 
the mapper found at parse time, and we can write it with either that 
mapper or the one we find at generation time.

> Another confusion we are already facing is when the actual option 
> become effective.
>
> For example generateStackmap(false) does not implicitly mean they will 
> be dropped by the transformation (unless we expand each Code 
> attribute). The same for processDebug, processLineNumbers and 
> patchDeadCode.
>

Correct, this is part of what I mean by the "let's review our options 
and make sure they mean what we think, and write that down" above.

> While DROP_DEBUG_ON_READis clearly an option, 
> DROP_DEBUG_ON_WRITEsuppose to be rather a CodeTransform, because by 
> setting the option we do not say it will be effective.
>
> All attributes attached to code have very specific position, as we 
> must expand the whole code when we want just filter them out.
>
> To your example I’m not quite sure what to expect from:
>
>     ClassModel cm = CC.of(DROP_DEBUG).parse(bytes);
>     CC.of(do not DROP_DEBUG).transform(cm, transform);
>

So, what happens here is that on reading, we drop the debug information, 
so the ClassModel has no debug information.  When we use that as a 
source, there's no debug information in the source but any debug 
information added by the transform is not dropped.

> *From: *Brian Goetz <brian.goetz at oracle.com>
> *Date: *Friday, 19 May 2023 16:15
> *To: *Adam Sotona <adam.sotona at oracle.com>, liangchenblue at gmail.com 
> <liangchenblue at gmail.com>, classfile-api-dev 
> <classfile-api-dev at openjdk.org>
> *Subject: *Re: Planned features of a classfile context object
>
> I agree that much of what such a context would manage is options.  But 
> I think we can also agree that the current treatment, where we stash 
> Options in the CPBuilder, isn't right either.
>
> I can buy that sometimes we want to parse with different options than 
> we generate with.  We can add this in as as goal for rationalizing the 
> classfile context.  I suspect also that part of the problem is that 
> our options are too granular: "process debug" is just a boolean, which 
> could be broken down further.  As a not-entirely-serious suggestion:
>
>     enum ProcessDebugOptions implements Option {
>         DROP_DEBUG, DROP_DEBUG_ON_READ, DROP_DEBUG_ON_WRITE;
>     }
>
> Originally all our options were booleans, and we've moved past that 
> structurally, but haven't really reexamined all the options we defined.
>
> I think the problem with "can't transform with different options" 
> stems from the fact that transform lives only on the model, not on the 
> context.  This would let you do what you want as:
>
>     ClassModel cm = CC.of(opts1).parse(bytes);
>     CC.of(opts2).transform(cm, transform);
>     CC.of(opts3).transform(cm, transform);
>
> This derives from the principle that all classfile entities are 
> immutable and can be used in any context; no need to re-parse.
>
>
>
> On 5/19/2023 3:52 AM, Adam Sotona wrote:
>
>     I think our current Classfile.Option is already not so far from
>     the proposed global context holder approach.
>
>     The most significant use cases to persist (class hierarchy
>     resolution and custom attributes) can be very well cached by
>     keeping the Classfile.Option instances and re-using them as many
>     times as user needs.
>
>     var options = List.of(Option.classHierarchyResolver(resolver),
>
>                           Option.attributeMapper(mapper));
>
>     Classfile.build(clsDesc, options, handler);
>
>     Classfile.parse(bytes, options); //here we have only vararg now
>
>     What I’m missing much much more than “global options holder” is
>     per-transformation options (or even option change for transformed
>     method).
>
>     For example I would like to use one class model to transform into
>     multiple classes with different options, and I have to write:
>
>     Classfile.parse(bytes, options1).transform(classTransform1);
>
>     Classfile.parse(bytes, options2).transform(classTransform2);
>
>     Instead of simple:
>
>     var clm = Classfile.parse(bytes);
>
>     clm.transform(classTransform1, options1);
>
>     clm.transform(classTransform2, options2);
>
>     Or maybe:
>
>     var clm = Classfile.parse(bytes, options1);
>
>     clm.transform(classTransform1);
>
>     clm.setOptions(options2);
>
>     clm.transform(classTransform2);
>
>     However it raises the main question about what options are really
>     tied to model parsing (I think it is only attributeMapper).
>
>     And what options can be safely changed later. For example can I
>     change processDebug, processLineNumbers or
>     processUnknownAttributes on already parsed class model to get
>     different results of the following model traversal?
>
>     And there are majority of options unrelated to the parsed class
>     model but affecting only building and transformations
>     (generateStackmap, constantPoolSharing, fixShortJumps,
>     patchDeadCode, classHierarchyResolver, filterDeadLabels).
>
>     I think we should make more clear which options affect what
>     actions (parsing, building, transformation) and give more
>     flexibility to change them on the fly.
>
>     Thanks,
>
>     Adam
>
>     *From: *classfile-api-dev <classfile-api-dev-retn at openjdk.org>
>     <mailto:classfile-api-dev-retn at openjdk.org>on behalf of Brian
>     Goetz <brian.goetz at oracle.com> <mailto:brian.goetz at oracle.com>
>     *Date: *Thursday, 18 May 2023 16:16
>     *To: *liangchenblue at gmail.com
>     <mailto:liangchenblue at gmail.com><liangchenblue at gmail.com>
>     <mailto:liangchenblue at gmail.com>, classfile-api-dev
>     <classfile-api-dev at openjdk.org> <mailto:classfile-api-dev at openjdk.org>
>     *Subject: *Re: Planned features of a classfile context object
>
>     Currently the ConstantPoolBuilder is another place we we attach
>     options.  This was more of a sin of convenience than anything
>     else; the theory was that if we are transforming a class, we'll
>     parse it with a set of options, and then generate a new class
>     which shares its CP, and the options came along for the ride.
>
>     Searching for a better name that ClassfileContext.  The name
>     Classfile is too good to waste, but it doesn't represent a
>     classfile (that's ClassModel), so calling the new thing Classfile
>     would likely be weird.  Also ClassfileContext would make it harder
>     to discover the key entry points (build/parse).  Classfile.Context
>     is better in that it is more discoverable at least.
>
>     Classfile{Parser,Generator} are more discoverable but only tell
>     half the story, and there's no obvious "first half" of the story. 
>     ClassfileReaderWriter is discoverable and honest but long. 
>     Classfile{Broker,Manager,Mediator} sound like parodies of the
>     Design Patterns era.
>
>     A slightly cheeky but possibly viable option is "Classfiles";
>     while not a collection of classfiles, it is a collection of
>     behaviors _about_ classfiles.
>
>         Classfiles.of(options).parse(bytes)
>         Classfiles.of(options).generate(handler)
>
>
>
>
>
>     On 5/17/2023 11:21 AM, Brian Goetz wrote:
>
>         These are basically what is on my list, though I had in mind
>         to move _all_ option information to the context, and leave it
>         out of individual processing decisions.
>
>         On 5/17/2023 9:59 AM, - wrote:
>
>             Hi,
>
>             In the discussions a few weeks ago, we envisioned a Classfile context
>
>             object shared across multiple Classfile processing scenarios, mainly
>
>             for Class hierarchy information caching.
>
>               
>
>             I want to confirm that these are contents of the object, mainly
>
>             promoted from individual options:
>
>               - Class Hierarchy information caching
>
>               - Custom attribute processing
>
>               - A set of default options, propagated to individual processing (may
>
>             be overridden classfile-wise)
>
>               
>
>             What else is planned for the context object currently? Please don't
>
>             hesitate to propose. Thanks!
>
>               
>
>             Chen Liang
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230519/5ca49877/attachment-0001.htm>