-
Notifications
You must be signed in to change notification settings - Fork 200
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
perf: Improve output speed by caching author_list #1090
Conversation
@fukusuket Thank you so much for the quick PR!
By the way, is the rule author list only needed for outputting author names next to rules in the HTML report? |
@YamatoSecurity
It was a necessary process even for terminal output.Therefore, it is necessary to calculate even without the |
@YamatoSecurity |
@fukusuket That is a good point! |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1090 +/- ##
==========================================
- Coverage 82.14% 82.13% -0.01%
==========================================
Files 24 24
Lines 19915 19918 +3
==========================================
+ Hits 16359 16360 +1
- Misses 3556 3558 +2
☔ View full report in Codecov by Sentry. |
I checked with the data of all-evtx.tgz (6.1GB). The results are as follows.
|
@fukusuket Thanks! If it is just 1 second then that is no problem. I will check it on a larger dataset. |
@fukusuket 悲報: Unfortunately there is not a big improvement for my larger dataset (32GB). Output time does decrease from 34 min 30 seconds to 34 min 10 sec, but 2.5.1 is only around 11 min. |
e031962
to
8689b7a
Compare
@hitenkoku if this PR is included in #1089 should we close it? |
Yes, it's included in #1088, so I'll close it! :) |
What Changed
This fix may not improve memory usage. In that case, it will be handled as a separate issue.
I would appreciate it if you could review🙏
Evidence
Environment
Test(all-evtx.tgz(6.1GB))
I confirmed that there is no difference result file and output speed improved.
main
This PR