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

[Tech-Debt]: Using a streaming implementation when searching in a file for a regex #433

Closed
shawn-hurley opened this issue Nov 16, 2023 · 7 comments
Labels
performance Analyzer performance related issues priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Comments

@shawn-hurley
Copy link
Contributor

We are worried about memory usage of the binary running an unbounded number of threads reading files implemented here

We also do the same thing in the generic provider, and we should consider moving both to streaming the files.

@shawn-hurley shawn-hurley added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. performance Analyzer performance related issues labels Nov 16, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Planning Nov 16, 2023
@pranavgaikwad pranavgaikwad moved this from 🆕 New to 📋 Backlog in Planning Nov 16, 2023
@Parthiba-Hazra
Copy link
Contributor

@shawn-hurley can I work on it?

@shawn-hurley
Copy link
Contributor Author

@Parthiba-Hazra

I would not be opposed to trying and getting this working again. To note: we had to revert the change in #430 because of negative look-ahead regex searches and we would need to solve that problem simultaneously.

I do not currently have a good way of doing that, so I worry about this.

It sounds like you are eager to take something and that is genuinely appreciated. Can I work with you on Slack because it is apparent that after the break and the drive for the release we may need to do some issue cleanup.

@Parthiba-Hazra
Copy link
Contributor

@shawn-hurley I see, I will reach out to you on slack, thanks for the clarification.

@shawn-hurley
Copy link
Contributor Author

Sorry again, I wish that I had something off the top of my head to give you, but I am just getting back into the swing of things after a break so I also need to look through things

@Parthiba-Hazra
Copy link
Contributor

@shawn-hurley no problem I can understand.

@shawn-hurley
Copy link
Contributor Author

I am considering closing this issue. We attempted this, but it is a problem because golang regex doesn't support negative look-ahead (IIRC). @pranavgaikwad @fabianvf @jmle can you advise?

@pranavgaikwad
Copy link
Contributor

@shawn-hurley Yes, this should be closed. not using go regex anymore

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Planning Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Analyzer performance related issues priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants