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

fix: downgrade mimalloc_rust version because of performance issue. #875

Closed
wants to merge 1 commit into from

Conversation

fukusuket
Copy link
Collaborator

@fukusuket fukusuket commented Jan 9, 2023

I don't know if it's a mimalloc_rust problem or a mimalloc problem, the speed is degraded as shown in the benchmark results below.(On the other hand, memory usage seems to have improved a little? ... 🤔)

What Changed

  • Downgrade mimalloc_rust/libmimalloc-sys versions.
    • because latest mimallc_rust(v0.1.34) seems to have some performance issue.

Evidence

Environment

  • OS: Windows 10 Home edition
  • Hard: Memory 16GB , Core 8, SSD, laptop

Benchmark

I ran a benchmark using this procedure(6.1GB evtx) and the results were as follows.

Version Elapsed time Memory(peak) Events with hits / Total events Output file size(bytes)
2.1.0 00:13:51.232 5.0 GiB 1,593,660 / 4,817,181 575217451
This PR 00:13:33.355 5.4 GiB 1,593,660 / 4,817,181 575217451

Console output

2.1.0

Results Summary:

Events with hits / Total events: 1,593,660 / 4,817,181 (Data reduction: 3,223,521 events (66.92%))

Total | Unique detections: 1,627,378 | 148
Total | Unique critical detections: 0 (0.00%) | 0 (0.00%)
Total | Unique high detections: 12,044 (0.74%) | 20 (13.51%)
Total | Unique medium detections: 11,267 (0.69%) | 38 (25.68%)
Total | Unique low detections: 1,053,568 (64.74%) | 40 (27.03%)
Total | Unique informational detections: 550,499 (33.83%) | 50 (33.78%)
...
Saved file: 2.csv (575.2 MB)
Elapsed time: 00:13:51.232
Rule Parse Processing Time: 00:00:20.664
Analysis Processing Time: 00:13:09.427
Output Processing Time: 00:00:21.139

Memory usage stats:
heap stats:    peak      total      freed    current       unit      count
  reserved:    5.0 GiB    5.0 GiB   83.0 MiB    4.9 GiB
 committed:    4.6 GiB   67.4 GiB   63.0 GiB    4.4 GiB

This PR

Results Summary:

Events with hits / Total events: 1,593,660 / 4,817,181 (Data reduction: 3,223,521 events (66.92%))

Total | Unique detections: 1,627,378 | 148
Total | Unique critical detections: 0 (0.00%) | 0 (0.00%)
Total | Unique high detections: 12,044 (0.74%) | 20 (13.51%)
Total | Unique medium detections: 11,267 (0.69%) | 38 (25.68%)
Total | Unique low detections: 1,053,568 (64.74%) | 40 (27.03%)
Total | Unique informational detections: 550,499 (33.83%) | 50 (33.78%)
...
Saved file: 1.csv (575.2 MB)
Elapsed time: 00:13:33.355
Rule Parse Processing Time: 00:00:19.576
Analysis Processing Time: 00:12:52.228
Output Processing Time: 00:00:21.548

Memory usage stats:
heap stats:    peak      total      freed    current       unit      count
  reserved:    5.4 GiB    5.4 GiB   56.0 MiB    5.3 GiB                        not all freed!
 committed:    4.7 GiB    6.2 GiB    1.5 GiB    4.6 GiB                        not all freed!

@fukusuket fukusuket changed the title fix: downgrade mimalloc version beacause of performance issue. fix: downgrade mimalloc_rust version beacause of performance issue. Jan 9, 2023
@YamatoSecurity
Copy link
Collaborator

Thank you! I will check this on my end as well.

@fukusuket fukusuket changed the title fix: downgrade mimalloc_rust version beacause of performance issue. fix: downgrade mimalloc_rust version because of performance issue. Jan 9, 2023
@YamatoSecurity
Copy link
Collaborator

I did some benchmarks on my end.
It seems it is 2% slower but uses 2% less memory.
I think I am going to keep it at 0.1.34 for now as there apparently were some bug fixes and 2% is not a huge difference.
I submitted an issue here for now:
purpleprotocol/mimalloc_rust#89
Let's see what the say and if they can fix it.
We might just have to trust upstream MS that whatever improvements they are making are worth the degradation in performance.

@fukusuket
Copy link
Collaborator Author

Thank you for the benchmark and issue report 🙇
Yes, I'll trust upstream improvements :) Close this PR

@fukusuket fukusuket closed this Jan 10, 2023
@fukusuket fukusuket deleted the downgrade-mimalloc_rust-improve-performance branch January 10, 2023 09:16
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