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

Add compiler profile to prioritize memory speed #167

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vthib
Copy link
Owner

@vthib vthib commented Jul 30, 2024

  • Add a new CompilerBuilder object, use it to improve the API of adding modules
  • Add a CompilerProfile enum, so that memory usage can be selected instead of scanning speed

TODO: would be nice to run all tests on both profiles

Add a builder for the compiler object. This builder is only used
for the moment to add modules, but more will be added.

This builder solves an issue when adding modules, in that existing
modules could not be replaced by the one added, which made for a much
worse API and the impossibility of overwriting a module configuration
with a new one.

This is now solved as with a CompilerBuilder, it is guaranteed that
added modules have not yet been used by any rules, so they can be
replaced.

This makes creating a compiler a bit more complex when custom modules
are added, but this is not a huge change.
This avoids the "too many arguments" lint on Scanner::new, and will
scale better.
Add a compiler profile option to CompilerBuilder. This will allow
picking between:

- speed: prioritize scanning speed (current impl & default)
- memory: prioritize memory usage
- automatic: pick between the two depending on the size of the rules.

The memory profile is only implemented for the aho-corasick for the
moment. Changing the regex engine used for validators could be an
option, however it isn't trivial to do, and we would make implementing
an "automatic" profile harder, since "automatic" only works if
compilation is done when converting the compiler to the scanner.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant