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

Fix for #15172 introduces significant performance degredation #15324

Closed
Mnkras opened this issue Jun 29, 2024 · 2 comments
Closed

Fix for #15172 introduces significant performance degredation #15324

Mnkras opened this issue Jun 29, 2024 · 2 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@Mnkras
Copy link

Mnkras commented Jun 29, 2024

Describe the bug
In commit 7288fec the assertion on the input to check if the input was a valid ip was "fixed" to make it more complete, unfortunately the additional checks significantly increase CPU time which was unexpected.

It seems that the new assertions are more computationally expensive than the actual matching code.

To Reproduce
Run sample below for a basic test showing the difference is performance

Expected behavior
No significant performance regressions

Sample

(https://gist.github.com/Mnkras/b02bd4e68cc4455d1aed6b2dde8c411f)

@jzheaux FYI

@Mnkras Mnkras added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 29, 2024
@jzheaux jzheaux self-assigned this Jul 1, 2024
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 1, 2024
@jzheaux jzheaux added this to the 6.3.2 milestone Jul 1, 2024
@jzheaux jzheaux closed this as completed in 8917cdb Jul 1, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Jul 1, 2024

Thanks for the report, @Mnkras. I've made some performance improvements. On my machine, the ratio drops from ~500 to ~10. Can you please try the 6.3.2-SNAPSHOT and let me know?

@Mnkras
Copy link
Author

Mnkras commented Jul 1, 2024

That looks like it will be way faster, for cases where you know the input will be valid, would it make sense to add a flag to skip the assertions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
Status: No status
Development

No branches or pull requests

2 participants