-
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
chg: build.rs(for vc runtime) to rustflags in config.toml and replace default global memory allocator with mimalloc. #777
chg: build.rs(for vc runtime) to rustflags in config.toml and replace default global memory allocator with mimalloc. #777
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.
@fukusuket Thank you for your pull request!
This improve is great.
LGTM
I also compared the speed on macOS. There seems to be no regression, but no improvement either :( Environment
Speed test results.
|
Unfortunately in github Actions build (windows-latest, i686-pc-windows-msvc, false) failed😭 |
@fukusuket sometimes cross compiling will fix it, so I set it to true. |
@fukusuket おっと。。(T_T) 残念ながらcross: trueでも駄目でした。。 |
ありがとうございます!🙇 crossオプションでもダメなのですね...涙 こちらでももう少し調査いたします! |
@fukusuket Now we are using |
@fukusuket Also, did you check that this correctly gets statically compiled and that it will run on systems without the VC++ Redistribution package? |
@YamatoSecurity |
…er-option' into chg-vc-runtime-buildrs-to-compiler-option
Evidence Environment
Test4Compare the below execution speed(3 times each).
|
Evidence Environment
Test5i686 build succeeds as follows.
Sorry, Because there is no 32-bit environment, I couldn't check the actual execution...🙇 |
Evidence Environment
Test6Compare the below execution speed(3 times each).
|
Testing in my environment is complete. I would appreciate it if you could review🙏 |
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.
Ok. I checked added code.
LGTM
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.
mimallocの設定はとても悩ましいですが、取り敢えずパフォーマンスを優先したいので、default-featuresをfalseに変えました。
PRありがとうございました!
パフォーマンス優先とのこと承知いたしました! |
Thank you so much for review and advice :) |
What Changed
build.rs
(static_vcruntime crate) and set rustc compile option in ./cargo/config.toml.build.rs
.rustflags
are described #657 issue comment.Evidence
Environment
Test1
Compare the below execution speed of ver1.7.2 and the fixed version 3 times each.
.\hayabusa.exe -d C:\tmp\hayabusa-sample-evtx -o out.csv
then execution time is as follows.
Test2
Compare the detection results ver1.7.2 and the fixed version.
I confirmed that the number of detections is the same.
ver1.7.2
fixed version
I would appreciate it if you could review🙏
closes #657