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

Minor code cleanup: ternary if statement and lambda #692

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

hazendaz
Copy link
Collaborator

No description provided.

@hazendaz hazendaz self-assigned this Dec 21, 2023
@hazendaz
Copy link
Collaborator Author

@mathieucarbou On review, you will want to hide whitespace to see this easier.

@mathieucarbou mathieucarbou merged commit 981c386 into mathieucarbou:master Dec 21, 2023
12 checks passed
@mathieucarbou
Copy link
Owner

thanks for all these recent PRs 👍

@hazendaz
Copy link
Collaborator Author

@mathieucarbou Sure no problem, I'm trying to hunt down an issue where this plugin is incredibly slow and CPU spikes. I've had little luck tracking that down as of yet but everything else is just nice to have :)

For reference in case you are wondering, with https://github.com/mybatis/mybatis-3, it takes well over a minute to run through this plugin only with git checks on dates. That project is old and has significant git history so that is likely most of the reason but I suspect some leaks in the code potentially cause the problem. I do have a highly powerful machine and the behavior is very consistent on mine and has been enough that on most github projects as well as work projects, where possible we turn this plugin off due to this. I believe that is why there are general asks to get smarter about handing dates in general ;)

@mathieucarbou
Copy link
Owner

Pretty sure this is the issue: going through the commit range for the dates, and for each file, is really expensive.
The code is also not improved a lot and quite sequential: ideally, it would need to be refactored and use maybe a sort of work stealing queue with N threads, N being the number of cores of the machine, and be able to handle in one time a batch of files. This is a big rework though I think because all the current processing is per file.

@mathieucarbou
Copy link
Owner

Note: maybe create an issue for that, flagged as todo and bug ?

@hazendaz hazendaz added this to the 4.4 milestone May 6, 2024
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.

2 participants