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.

Sign up for GitHub

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

Jump to bottom

Stop incrementing jit_entry_calls once threshold is hit #8259

Merged
merged 1 commit into from
Aug 23, 2023
Merged

Stop incrementing jit_entry_calls once threshold is hit #8259

merged 1 commit into from
Aug 23, 2023

Conversation

byroot
Copy link
Member

@byroot byroot commented Aug 22, 2023 •
edited

Otherwise the ISeq page will constantly be written into preventing it from being shared.

cc @k0kubun as we mentioned that recently.

@byroot byroot requested a review from k0kubun August 22, 2023 08:42
@byroot
Copy link
Member Author

byroot commented Aug 22, 2023

Hum, RJIT tests are failing in a weird way, not sure why.

@k0kubun
Copy link
Member

k0kubun commented Aug 22, 2023 •
edited

You're returning body->jit_entry even when rb_rjit_call_p is false. rb_rjit_call_p is deliberately separated from rb_rjit_enabled to disable calling compiled code. When it's false, it must return NULL for invalidation reasons.

Also, with today's master, you'd want to change jit_compile_exception and body->jit_exception_calls as well.

@byroot
Copy link
Member Author

byroot commented Aug 23, 2023

When it's false, it must return NULL for invalidation reasons.

Ah thank you, I totally missed that.

@byroot byroot force-pushed the jit-compile-limit-writes branch 4 times, most recently from 72e7367 to 72072dd Compare August 23, 2023 08:20
@byroot byroot marked this pull request as ready for review August 23, 2023 08:59
@byroot
Copy link
Member Author

byroot commented Aug 23, 2023

@k0kubun ok, I think I got it correct now?

k0kubun
k0kubun approved these changes Aug 23, 2023
View reviewed changes
Copy link
Member

@k0kubun k0kubun left a comment

Choose a reason for hiding this comment

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

👍

k0kubun
k0kubun reviewed Aug 23, 2023
View reviewed changes
vm.c Outdated Show resolved Hide resolved
k0kubun
k0kubun reviewed Aug 23, 2023
View reviewed changes
vm.c Outdated Show resolved Hide resolved
k0kubun
k0kubun reviewed Aug 23, 2023
View reviewed changes
vm.c Outdated Show resolved Hide resolved
@byroot
Stop incrementing jit_entry_calls once threshold is hit
e6dbb21 
Otherwise the ISeq page will constantly be written
into preventing it from being shared.
@byroot byroot merged commit cedb333 into ruby:master Aug 23, 2023
90 checks passed
@byroot byroot deleted the jit-compile-limit-writes branch August 23, 2023 22:44
@casperisfine casperisfine mentioned this pull request Sep 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k0kubun k0kubun k0kubun approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
2 participants
@byroot @k0kubun