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

Improve performance by removing respond_to? from runtime code #45578

Merged
merged 1 commit into from Jul 12, 2022

Conversation

natematykiewicz
Copy link
Contributor

respond_to? is inherently slow, and the YAML class won't change whether
it does or does not respond to unsafe_load, so just check it once
during file load, and define methods accordingly.

cc @byroot

respond_to? is inherently slow, and the YAML class won't change whether
it does or does not respond to unsafe_load, so just check it once
during file load, and define methods accordingly.
@natematykiewicz
Copy link
Contributor Author

natematykiewicz commented Jul 12, 2022

Is this worth a x.x.x.2 release? I'm honestly unsure how big of a performance difference this is. It's sort of a regression fix for the security release that went out today. But maybe it's not enough of a deal to warrant a release.

@byroot
Copy link
Member

byroot commented Jul 12, 2022

Is this worth a x.x.x.2 release?

No it's not. The performance difference isn't that big to really matter that much. I'll merge tomorrow, but probably won't bother backporting.

@natematykiewicz
Copy link
Contributor Author

Makes sense. Thanks!

@rafaelfranca rafaelfranca merged commit 989de53 into rails:main Jul 12, 2022
@natematykiewicz natematykiewicz deleted the yaml_safe_load_performance branch July 12, 2022 21:28
rafaelfranca added a commit that referenced this pull request Jul 12, 2022
…ance

Improve performance by removing respond_to? from runtime code
rafaelfranca added a commit that referenced this pull request Jul 12, 2022
…ance

Improve performance by removing respond_to? from runtime code
eileencodes pushed a commit that referenced this pull request Jul 15, 2022
…ance

Improve performance by removing respond_to? from runtime code
eileencodes pushed a commit that referenced this pull request Jul 15, 2022
…ance

Improve performance by removing respond_to? from runtime code
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.

None yet

4 participants