Skip to content

Remove LockSupport implementation details from the trace#662

Open
Evgeniy Moiseenko (eupp) wants to merge 12 commits into
developfrom
eupp/park-trace-fix
Open

Remove LockSupport implementation details from the trace#662
Evgeniy Moiseenko (eupp) wants to merge 12 commits into
developfrom
eupp/park-trace-fix

Conversation

@eupp

Copy link
Copy Markdown
Collaborator

Resolves #627

…kingTrackerTransformer`.

Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
@eupp Evgeniy Moiseenko (eupp) changed the base branch from master to develop May 20, 2025 17:59
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
…t supported in mc yet)

Signed-off-by: Evgeniy Moiseenko <evg.moiseenko94@gmail.com>
| status ➜ 3 at AbstractQueuedSynchronizer$ConditionNode.isReleasable(AbstractQueuedSynchronizer.java:515) |
| LockSupport.park() at AbstractQueuedSynchronizer$ConditionNode.block(AbstractQueuedSynchronizer.java:519) |
| PARK at LockSupport.park(LockSupport.java:371) |
| PARK at AbstractQueuedSynchronizer$ConditionNode.block(AbstractQueuedSynchronizer.java:519) |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more intuitive to leave LockSupport.park() and hide PARK?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree.

But I suggest doing it in a separate PR, because:

  • we will need to change bytecode transformer and trace point logic;
  • for consistency, we also need to do this for other APIs, i.e., print Object.wait() instead of WAIT.

/**
* [ParkingTransformer] tracks [Unsafe.park] and [Unsafe.unpark] method calls,
* injecting invocations of [EventTracker.park] and [EventTracker.unpark] methods.
* The ParkingTransformer class is responsible for inserting bytecode instrumentation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it just a refactoring here?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the hiding logic is here, please consider doing it on the trace post-processing phase

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it just a refactoring here?

I changed the transformer to track both Unsafe and LockSupport part APIs.
Besides improving trace, tracking LockSupport directly also simplifies another thing in ManagedStrategy.kt (see the diff).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hide LockSupport implementation details in the trace

2 participants