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

Replace usage of range-checked 'at' method when vector/string has already been size checked #3280

Merged
merged 1 commit into from
Oct 19, 2024

Conversation

eduar-hte
Copy link
Contributor

what

Replace places in the codebase where the at method of a std::vector or std::string instance is used to access an element instead of the subscript operator ([]), and the index of the item is guaranteed to be valid.

Additionally, simplify some of these uses where an old-style C loop is used to iterate over all elements and replace it with a range-based loops (which also implicitly guarantees no out-of-bound access).

why

The at method is not equivalent to the subscript operator ([]) operator, as it's range-checked and can thus throw a std::out_of_range exception which makes the code slower not only because of the additional check but also due to the generation of exception handling code at these call sites.

Because the updated instances are guaranteed to access valid indexes in the container, we can replace the usage of at to improve performance.

And where possible, simplify the code by adopting ranged-based loops.

misc

This is part of a series of PRs to improve performance of the library (7/n). Previous: #3253

@eduar-hte
Copy link
Contributor Author

eduar-hte commented Oct 17, 2024

This is part of a series of PRs to improve performance of the library (7/n). Previous: #3253

These changes provide minor performance improvements to the library.

These results were obtained running the benchmark test with 100'000 iterations with the OWASP CRS v4 and are presented as a reference (average of three runs):

  • Mac mini (1st generation)
    • v3/master: 80.74 secs
    • This PR: 80.39 secs (0.44% improvement)
  • Debian (bullseye) container running on WSL on Windows Dev Kit 2023
    • v3/master: 119.01 secs
    • This PR: 114.87 secs (3.5% improvement)

Copy link

sonarcloud bot commented Oct 17, 2024

Copy link
Member

@airween airween left a comment

Choose a reason for hiding this comment

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

LGTM

@airween
Copy link
Member

airween commented Oct 19, 2024

Thanks @eduar-hte!

@airween airween merged commit ec506da into owasp-modsecurity:v3/master Oct 19, 2024
49 checks passed
@eduar-hte eduar-hte deleted the range-checked-at branch October 19, 2024 13:21
@marcstern marcstern added the 3.x Related to ModSecurity version 3.x label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants