-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
dev: processor optimizations #5316
Conversation
72b2aba
to
3215445
Compare
4642d9e
to
2e4d3d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Re IdentifierMarker
, I think it's a bit weird. It feels like it's from early days where the linters and their diagnostics was more controlled. Surely I'd like to have identifiers within backtiks just as much as the next person, but having a processor doing search replace on hand written regexes for each linter feels a bit unnecessary and hard to maintain. I'm sure there are more identifiers out there we don't replace, or replacements that are removed from the upstream linters anyways.
I agree, we will not remove it for now, I want to avoid "breaking changes" but we will not add new regexp unless there is a real need. |
I was working on the new exclusion configuration #5297, and I found some "optimizations".
IdentifierMarker
processor update:PathAbsoluter
processor:Cgo
processor and theFilenameUnadjuster
then now thePathAbsoluter
does that once.analysis.Diagnostic
are absolute.PathPrettifier
(it already exists) converts absolute path to relative (related to the different elements but mainly to exclusions)This is not only "optimizations", the
PathAbsoluter
is something I will use with the new exclusion system.I found another optimization: the usage of
FileCache
insidemisspell
.The
FileCache
is used bySourceCode
,ExcludesRules
,severity
,Fixer
processors, andmisspell
.The
FileCache
is loaded with file content and the keys are paths, butmisspell
is the only one that uses absolute paths to load/read data from/to the cache.So the cache is filled but never used.
This allows to reduce the memory usage (ex: on k8s this 50MiB)
These are minor optimizations: a few seconds, but they can impact large projects.
To avoid creating a PR for exclusions with some off-topics, and as those elements are standalone, I prefer to create a PR with just these processors.