RFR: 8293170: Improve encoding of the debuginfo nmethod section [v11]

[Boris Ulasevich]

On Wed, 16 Nov 2022 00:57:02 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> src/hotspot/share/code/compressedStream.hpp line 195:
>> 
>>> 193:     return _position;
>>> 194:   }
>>> 195:   void set_position(int pos) {
>> 
>> Now I see why we have a problem to implement `position` and `set_position`.
>> `position` originally had a meaning of the position where data would be written. Because of this it could be used to get the total amount of data written (see `DebugInformationRecorder::data_size`).
>> It was also used to mark a position to roll back later (e.g. `DebugInformationRecorder::serialize_scope_values`).
>> This violates the single-responsibility principle and makes difficult to add another implementation.
>> To restore the principle we need separate functionalities from `position` and `set_position` into something like:
>> 
>> // Mark the state of the stream.
>> void mark();
>> 
>> // Roll the stream state back to the marked one.  
>> void roll_back();
>> 
>> // Return the amount of data the stream contains.
>> int data_size();
>> 
>> 
>> We implement `mark` as creating copies of `_position`, `_bit_position` and `_buffer[_position]`. `roll_back` uses the copies to restore the state of the stream.
>> `CompressedSparseDataWriteStream::data_size()` just returns `_position + 1`.
>> 
>> There is the problem with `DebugInformationRecorder::find_sharable_decode_offset(int stream_offset)`. It calculates `stream_length` using `position()`. It depends too much on the current implementation. Because of this dependency we have to emulate it in our new implementation.
>
> I have an idea which might solve the issues.

> // Roll the stream state back to the marked one.  
> void roll_back();

get_position() is not about roll back only. See the DebugInformationRecorder, it serializes offsets into the stream.


int DebugInformationRecorder::serialize_scope_values(...) {
  ...
  int result = stream()->position();
  ...
  return result;
}

DebugToken* DebugInformationRecorder::create_scope_values(...) {
  ...
  return (DebugToken*) (intptr_t) serialize_scope_values(values);
}

void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) {
  ...
  DebugToken *locvals = C->debug_info()->create_scope_values(locarray);
  DebugToken *expvals = C->debug_info()->create_scope_values(exparray);
  DebugToken *monvals = C->debug_info()->create_monitor_values(monarray);

  C->debug_info()->describe_scope(
    ...
    locvals,
    expvals,
    monvals
  );
  
void DebugInformationRecorder::describe_scope(...
                                              DebugToken* locals,
                                              DebugToken* expressions,
                                              DebugToken* monitors) {
  ...
  // serialize the locals/expressions/monitors
  stream()->write_int((intptr_t) locals);
  stream()->write_int((intptr_t) expressions);
  stream()->write_int((intptr_t) monitors);

-------------

PR: https://git.openjdk.org/jdk/pull/10025