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

More perf #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

More perf #1

wants to merge 2 commits into from

Conversation

slang25
Copy link

@slang25 slang25 commented Oct 31, 2021

Before

Method Mean Error StdDev
Compress_Les_Misérables 129.10 ms 1.480 ms 1.236 ms
Compress_War_of_the_Worlds 14.35 ms 0.231 ms 0.247 ms
Method Mean Error StdDev
Decompress_Les_Misérables 95.236 ms 1.5021 ms 1.3316 ms
Decompress_War_of_the_Worlds 9.778 ms 0.1480 ms 0.1155 ms

After

Method Mean Error StdDev
Compress_Les_Misérables 112.91 ms 2.216 ms 1.730 ms
Compress_War_of_the_Worlds 14.10 ms 0.145 ms 0.129 ms
Method Mean Error StdDev Median
Decompress_Les_Misérables 94.826 ms 1.8838 ms 4.5134 ms 93.654 ms
Decompress_War_of_the_Worlds 9.969 ms 0.1972 ms 0.3705 ms 9.816 ms

@stevehjohn
Copy link
Owner

Nice @slang25. Looks like a near 10% improvement on compression.

Unfortunately, it seems to have slowed down decompression. Will figure out which bit did that and then get this merged.

On my machine:

Before

Method Mean Error StdDev
Compress_Les_Misérables 103.56 ms 1.155 ms 0.965 ms
Compress_War_of_the_Worlds 12.75 ms 0.179 ms 0.168 ms
Method Mean Error StdDev
Decompress_Les_Misérables 81.255 ms 1.1592 ms 1.0843 ms
Decompress_War_of_the_Worlds 9.855 ms 0.0753 ms 0.0668 ms

After

Method Mean Error StdDev
Compress_Les_Misérables 93.58 ms 1.104 ms 1.033 ms
Compress_War_of_the_Worlds 10.54 ms 0.145 ms 0.136 ms
Method Mean Error StdDev
Decompress_Les_Misérables 90.998 ms 0.8235 ms 0.7703 ms
Decompress_War_of_the_Worlds 9.682 ms 0.0376 ms 0.0333 ms

@stevehjohn
Copy link
Owner

Must be the ArrayPool as that's the only change you made for decompression (apart from the avoid double read, which does register as an improvement).

@stevehjohn
Copy link
Owner

stevehjohn commented Oct 31, 2021

Also (and this is just being really nitpicky now), the static members of the PriorityQueue would prevent it from being used on different types of object in the same application domain. In this project, that'll never be the case. Just sayin' that were PriorityQueue in a library to be used by different projects (which it won't), it could be an issue.

@stevehjohn
Copy link
Owner

Lastly, there's a R# Possible 'null' assignment to non-nullable entity on those static delegates. My anal-ness won't let that in, sorry! 🤣
Thanks for getting involved - great improvement on compression where I thought there were no more to be made.

@stevehjohn
Copy link
Owner

Can you add me as a collaborator so I can push a couple of minor changes to this PR please?

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