Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Panache bytecode enhancers to benefit from class transformers caches #40192

Merged
merged 1 commit into from
May 13, 2024

Conversation

Sanne
Copy link
Member

@Sanne Sanne commented Apr 22, 2024

@Sanne Sanne requested a review from FroMage April 22, 2024 14:11
@Sanne Sanne added the area/hibernate-orm Hibernate ORM label Apr 22, 2024
@Sanne Sanne force-pushed the BytecodeTransformersCaches branch 2 times, most recently from eb73ea1 to 1cefb15 Compare April 23, 2024 12:39
@Sanne Sanne force-pushed the BytecodeTransformersCaches branch from 1cefb15 to c73fc8c Compare May 3, 2024 16:33
@Sanne Sanne marked this pull request as ready for review May 3, 2024 16:33
@Sanne
Copy link
Member Author

Sanne commented May 3, 2024

"Undrafted" and rebased, let's have CI opinions as well.

I described the rationale above and think it should be safe after having convinced myself while writing the rationale :) but let's have @FroMage review it please.

This comment has been minimized.

@Sanne Sanne force-pushed the BytecodeTransformersCaches branch from c73fc8c to dc0c2b5 Compare May 8, 2024 20:29
@Sanne Sanne requested a review from FroMage May 8, 2024 20:30
@Sanne
Copy link
Member Author

Sanne commented May 8, 2024

Updated & rebased:

  • Reverted the second change; so we'll cache the transformation of entities which seems safe and also gives us the strongest benefits as it will allow to skip the other enhancements from Hibernate ORM as well, which are heavy.
  • Added a note linking to the above caveats in the code for the second transformation suggesting what one could do eventually, if we need to come back to this.

Copy link

quarkus-bot bot commented May 8, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit dc0c2b5.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@Sanne Sanne dismissed FroMage’s stale review May 9, 2024 09:54

Amended the PR to not cache the client-side transformations

Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

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

Yeah, that seems safer, I suppose transforming the accessors is only local-class dependent.

@FroMage FroMage merged commit 77cf0a8 into quarkusio:main May 13, 2024
40 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 13, 2024
@Sanne Sanne deleted the BytecodeTransformersCaches branch May 14, 2024 13:53
@Sanne
Copy link
Member Author

Sanne commented May 14, 2024

Pushed a follow-up as #40629

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

Successfully merging this pull request may close these issues.

4 participants