-
Notifications
You must be signed in to change notification settings - Fork 203
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
Improve speed #984
Improve speed #984
Conversation
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.
In my environment the speed (and memory usage, detection result file) seemed to be the same before and after the fix.(6.1GB) But no regressions, so LGTM!!🚀
@hitenkoku ありがとうございます!CSV出力しかまだテストできていないのですが、こんな感じです: PR: メモリ使用が300MB減っているものの、処理時間が少し増えているので、ゆっくり確認したいと思います。 @fukusuket 時間がある時に、確認をお願いできますか? |
@YamatoSecurity お忙しい中、ご確認ありがとうございます。StringからCompactstrに変更しているためその分遅くなったかもしれませんあります。(その代わりメモリ使用量が減った) |
(すみません🙇 main -E/This PR -Eの結果が間違っていたようなので、再取得して修正しました) |
@fukusuket ありがとうございます!filterとかでCompactStringにしているところをStringにrevertしたpull reqをべつに出しておきますねー |
This reverts commit 096698a.
#985 で作っておきました。 |
…eb4f59f3164229a6bae0d49 Revert "perf(filter, yaml): replaced String with ComapctString"
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #984 +/- ##
==========================================
- Coverage 75.96% 75.81% -0.15%
==========================================
Files 24 24
Lines 17221 17178 -43
==========================================
- Hits 13082 13024 -58
- Misses 4139 4154 +15
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
もう一回試しましたが、まだ |
@YamatoSecurity
コード差分から1分の速度差を出しそうな箇所を見つけられていないのですが、上記の条件次第で、速度差がでやすいということかもしれません...🤔 |
c1ef6ea でProfile内のCompactStringをCowに置き換えることでもう少し早くなりました。ご確認いただけますでしょうか。 6.1GBのevtxでWIndows11の環境で得られています
|
ちゃんとエビデンスを残さなくてすみません! 出力ファイルサイズと検知数は一緒です。 @hitenkoku 後で、もう一回調べてみますね。少々お待ち下さい。 @fukusuket ベンチマークを取る時にオプション、プロファイル等を決めた方が良さそうですね。何が良いか考えてみますね。 |
ご確認ありがとうございます!Cowの利用にあたり、一部処理でtoStringを一時的に作らざるを得ないというところとCowの生存期間の問題もありそうなのでもう少し方法ないかなーとはおもっています。あとはreplaceとかの処理についてもAho-Corasickを使えそうなところはあるのでそこも入れられると良さそうですかね…… |
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.
速度が約3〜5%増えています。LGTM!
Thank you!
確認が遅く、すみません🙇 こちらの環境でもスピードアップ確認いたしました!LGTM!🚀
|
ありがとうございます。CoWについてはstaticでやらないと難しそうなので、一旦この状態でスピードアップ対応としてマージをさせていただきます |
What Changed
Evidence
Benchmark by 6.1GB Evtx
main json-timeline(Elapsed time: 00:04:41.829)
This PR json-timeline(Elapsed time: 00:04:14.211)