RFR: Classfile API stack maps processing

[Adam Sotona]

I see it a bit opposite.

  1.  Processing stack maps is extremely advanced feature and 99.9% of users don’t want to touch it, so why PROCESS_STACK_MAP is off by default and GENERATE_STACK_MAPS is on by default
  2.  When the 0.1% user needs to process the stack maps, he needs full control over it. So when user enables PROCESS_STACK_MAP, full control of what drops to code builder is up-to user.
  3.  Most of the #2 cases are complex transformations (3-way merges, instrumentations, etc…) and per-method control is important. For example jdk.jfr class instrumentation involve all cases:
     *   Some methods from the first (shared CP) source are unchanged and so original stack maps can be reused
     *   Some methods from the second source (non-shared CP) are also unchanged, so inflated stack maps can be passed down the transformation
     *   Some methods are combined from both sources, so user may need to manually combine stack maps or just little adjust existing maps
     *   Some methods may be synthetized from scratch and generated stack maps are first choice

By dropping PROCESS_STACK_MAP switch and keep it always on we would slow down a bit parsing for 99.9% cases.
GENERATE_STACK_MAPS switch as the only one would not allow per-method individual handling.

If we want to reduce number of switches I would rather drop GENERATE_STACK_MAPS as the StackMapTableAttribute is mandatory and when user does not provide/transform its own – we should generate it.

From: Brian Goetz <brian.goetz at oracle.com>
Date: Wednesday, 17 August 2022 18:25
To: Adam Sotona <adam.sotona at oracle.com>, classfile-api-dev at openjdk.org <classfile-api-dev at openjdk.org>
Subject: Re: RFR: Classfile API stack maps processing
I'd like to be a little more aggressive on that: if generation is not disabled, then any stack map sent downstream is dropped on the floor, and either the original stackmap reused, or a new one generated.  Generating stack maps is definitely an advanced, error-prone feature; users should have to turn it on.

Given this, though, I think we can drop the PROCESS_STACK_MAP attribute, and always send it downstream, as this costs us almost nothing and the user can ignore it.

We should definitely not generate Code attributes with more than one stack map attribute in them.
On 8/17/2022 12:02 PM, Adam Sotona wrote:
Yes, user can send the stack maps to the builder anytime and it will disable generation for the actual method.
However user has to explicitly construct the attribute:
cob.with(StackMapTableAttribute.of(List.of(StackMapFrameInfo.of(myLabel, locals, stack), …));

Generation switch works independently, so user can keep the generation on for some methods and explicitly specify/transform StackMapTableAttribute for other methods of the same class.


From: Brian Goetz <brian.goetz at oracle.com><mailto:brian.goetz at oracle.com>
Date: Wednesday, 17 August 2022 17:53
To: Adam Sotona <adam.sotona at oracle.com><mailto:adam.sotona at oracle.com>, classfile-api-dev at openjdk.org<mailto:classfile-api-dev at openjdk.org> <classfile-api-dev at openjdk.org><mailto:classfile-api-dev at openjdk.org>
Subject: Re: RFR: Classfile API stack maps processing
See the RFR, I think I found a place where we might end up putting TWO stack map attributes into the builder.

With this patch, I believe the treatment of stack maps is intended to be:

   if (generation not disabled) {
       try to reuse original stackmap, if transforming and code array/exception table is unchanged
       otherwise generate stack map
   }
   else {
       if a stack map has been sent to the builder, put that in the class file
   }

I am not sure this is the effect, though; since the user can send stack maps down the pipe and they are stored as attributes regardless of options, I think these may be written to the classfile even if we are in generation mode.


On 8/17/2022 11:45 AM, Adam Sotona wrote:
Hi,
I would like to ask for review of proposed change in the Classfile API stack maps processing.
Primary motivation is to better align stack maps processing with the Classfile API and to allow manual constructions and transformations of stack maps.
Pull request for review is here: https://github.com/openjdk/jdk-sandbox/pull/32
And it includes following changes:

1.      New boolean Classfile.Option::processStackMaps with default to false has been added

2.      StackMapTableAttribute become a CodeElement processed when the above option is explicitly enabled by user

3.      StackMapTableAttribute, StackMapFrameInfo, VerificationTypeInfo, SimpleVerificationTypeInfo, ObjectVerificationTypeInfo and UninitializedVerificationTypeInfo have been simplified, refactored to use Labels instead of offsets and equipped with relevant static factory methods

4.      Redundant content of the StackMapFrameInfo and its sub-classes reflecting compressed frame forms have been removed

5.      UnboundStackMapTableAttribute with proper serialization has been implemented

6.      CorpusTest with RebuildingTransformations has been extended to process stack maps using the new API

New stack maps processing option affects inflation of stack maps and appearance of StackMapTableAttribute in the CodeElement stream.
Enabled stack maps processing inflates and decompresses StackMapTableAttribute with translated offsets to Labels and pass it to CodeTransformation(s).
User can individually filter StackMapTableAttribute out or transform it manually when explicitly enabled stack maps processing.
User-provided or transformed StackMapTableAttribute passed to CodeBuilder overrides stack maps generation process for the actual method.
Compressed source form of the parsed stack map frame can be identified from StackMapFrameInfo::tag
All new created StackMapFrameInfos always represent full frames with complete list of locals and stack.
Compression of StackMapTableAttribute frames is contextual operation and it is applied at the writing phase.
Any NO-OP transformation of inflated and decompressed StackMapTableAttribute may result in a different compressed form.
Performance boost tricks by-passing stack maps generation for unchanged methods are unaffected by this enhancement.

Thanks for your comments,
Adam





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20220817/1d885a81/attachment-0001.htm>