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

Optimize memory usage #1106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hurricup
Copy link

  • use more memory effective BitSet instead of boolean array
  • don't create a buffer until we really need it

- use more memory effective BitSet instead of boolean array
- don't create a buffer until we really need it
@ice1000
Copy link

ice1000 commented May 14, 2024

Unfortunately the project is dead :( can we have JetBrains jflex published to maven central?

@lsf37
Copy link
Member

lsf37 commented May 14, 2024

Not sure which project you mean is dead, but I'm not going to add an Emitter change with just the explanation "optimisation" without any benchmarks, use case or anything else provided.

For small scanners this is going to be slower, so from which size on is it better? Is it actually ever better? How much?

@ice1000
Copy link

ice1000 commented May 15, 2024

Not sure which project you mean is dead

Ohh!! I'm happy to see you're still active. I assumed this project is no longer receiving updates due to the amount of time it is being inactive.

@lsf37
Copy link
Member

lsf37 commented May 15, 2024

Still around, just not always with a lot of time. Sorry I was sounding a bit annoyed, PRs with subtle changes to the runtime engine like this one take an inordinate amount of time to review properly compared to the benefit they might (or might not) bring, so I've put it off for quite a while now.

@hurricup
Copy link
Author

@lsf37 why is it going to be slower?
This was inspired by memory snapshots analysis from intellij idea, which uses slightly modified version of jflex for its lexers.

  1. No need to allocate buffers if they are never used
  2. Bitset is more memory effective than boolean array

Not sure what benchmarks would you like to see. I did measured performance as well on some huge samples with warmup and stuff, and difference was neglectable, not remember in which direction. Like a percent or so.

This change is in the intellij fork already, so just wanted to contribute for the common good. If you don't think it is worth it, feel free to drop this PR, i'm totally ok with that.

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.

None yet

3 participants