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

Benchmarks and Unit tests? #50

Open
JimBobSquarePants opened this issue Apr 23, 2020 · 8 comments
Open

Benchmarks and Unit tests? #50

JimBobSquarePants opened this issue Apr 23, 2020 · 8 comments

Comments

@JimBobSquarePants
Copy link

Hi, Sorry if this is not the best place to ask questions but I'm very curious about this library and how it compares in both performance and compression to other implementations.

Do you have unpublished benchmarks and unit tests to accompany the source code?

@AraHaan
Copy link

AraHaan commented Apr 27, 2020

oh sorry I have not came around to benchmark or adding unit tests for this project atm. I was hoping of somehow auto generating unit tests for this like what rosyln does. However it seems IntelliTest is net framework only that auto generates unit tests. 😭 I think microsoft needs to add support for it for sdk style projects that targets frameworks like mine does.

For my XmlAbstraction package/project I use Xunit, however it seems Xunit does not like testing on netstandard2.0 for their visual studio adapter package, but on net40 the console and test adapter for vs resolves but the main Xunit package does not support it so I am ran across a cliff on needing to test all supported TargetFrameworks (for best code coverage to capture and dead or unused code that is #ifd based on specific TargetFrameworks).

I been planning on eventually adding unit tests and benchmarks for all of my packages though.

Sorry that I just now seen your issue, it somehow got lost in my notifications as I normally look at the ones I am participating in to see if anything updated.

Also because of that I been basically using this library in a ton of local projects to the point where I would know if something is wrong on this library from those, and one of those projects actually unpacks like about 12 GB worth of compressed data or so RIP. There was 1 time I screwed up something when I tried to fix a compile warning where all of the data would output at the correct unpacked length but all bytes would be null because every time it wanted me to use List instead of array and so every time it would create a new array (until I fixed that).

Oh wow you also work on xplat drawing stuff for net core, I am sure going to need that if I want to make an .net core only internet browser for windows, mac, and linux using the same code without any conditional windows junk. I know for sure I need in that case an html renderer of some kind, some way to render it to a window with tabs, theme the entire thing, js support, apng, all the other things normal browsers supports but also be super fast and more memory efficient than chrome, firefox, etc.

@JimBobSquarePants
Copy link
Author

Hi @AraHaan no need to apologize. You're under no obligation to reply at all 😄

XUnit cannot run tests against net standard since it needs to run against an implementation of the standard, not the standard itself. By targeting multiple implementations you should be able to cover most things.

Regarding target frameworks, it looks like you target a lot of frameworks, some of which aren't even supported by Microsoft. From my experience it's best to target the least number of frameworks possible and only add when there are specific enhancements that can be added.

I see a dependency on some GitBuildInfo which is influencing your target framework count. There's better alternatives out there for determining build numbers that don't require including in the source. For example, in the ImageSharp codebase we use GitVersion

https://github.com/SixLabors/ImageSharp/blob/f6122dfe41ff8084e2bee2b9ad061beec542e99c/.github/workflows/build-and-test.yml#L63

As far as unit tests go I would probable use the Zlib tests from SharpZipLib and the DeflateStream tests from the dotnet runtime as inspiration.

https://github.com/icsharpcode/SharpZipLib/tree/f36ca373206340a25b1d3bb226acc7b679128a7c/test

https://github.com/dotnet/runtime/blob/1cfa461bbf071fbc71ceb5e105e1d39d0c077f25/src/libraries/System.IO.Compression/tests/CompressionStreamUnitTests.Deflate.cs

I'll have a deeper look when I can to see if I can get some tests added.

@AraHaan
Copy link

AraHaan commented Apr 27, 2020

Ye, I made the GitBuildInfo package before to allow code to determine if they are using a dirty source tree or clean one, and to also determine if it is a master branch build or not as well.

@JimBobSquarePants
Copy link
Author

Think I've found an issue with the inflater btw.

Roundtrip deflate/inflate fails on the inflater side with larger buffers, returning zeros after array[504] of a decompressed stream given a 4*4096 input buffer to compress/decompress. The original ZLib.NET implementation doesn't seem to have this issue but first 7 bytes of the expanded buffer appear to be incorrect in that implementation so it's hard to compare what is going on.

@JimBobSquarePants
Copy link
Author

Actually, turns out the issue is present in both libraries.

while (this.Z.AvailOut == len && err == ZlibCompressionState.ZOK);

Should be

while (this.Z.AvailOut > 0 && err == ZlibCompressionState.ZOK); 

@AraHaan
Copy link

AraHaan commented Apr 28, 2020

oh crud I see it now.

Btw, is the same issue in ZOutputStream or anywhere else too?

@AraHaan
Copy link

AraHaan commented Apr 28, 2020

Alright I pushed the fix earlier.

@AraHaan
Copy link

AraHaan commented Apr 28, 2020

It seems the pr adds 2 warnings to the build I would like resolved or silenced.

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

No branches or pull requests

2 participants