-
Notifications
You must be signed in to change notification settings - Fork 15
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
wip: spanification and major rewrite draft #8
Conversation
f9da1c5
to
b1071a1
Compare
c16c861
to
331d9f1
Compare
331d9f1
to
bc075db
Compare
Most of the work done. Now need rework the most used surface of the library: the initial parsing with |
Hello, I wasted few hours of my time to make my own "spanification" (read-only, only tags and abc file) with a benchmark for the three available implementations (yours, mine and the default stream-based one) and figured out that unless we find a workaround the needed re-allocation to decompress ZLib data we won't beat the original implementation in term of allocations. Which is a shame ngl, and it seems that it won't be worked on cf. dotnet/runtime#16923 I won't be working further on the project since the original is okay but you can find the results and repo here : thenameless314159/DotNetFlashDecompiler#1 |
Hey, your version is awesome! I was also considering to use The compression being inside the file format is very annoying and painful. I removed LZMA compression support from the library prematurely when I saw .NET team prioritizing a inbox solution for it in .NET 7 IIRC but that got dropped AFAIK and pushed into the "Future" limbo. I have pretty much given up on all The original wrapped |
Unfortunately I had written most of the ABC file parsing logic already before making the bench, i would have saved a lot of time if I had made it earlier :/ Indeed but it was written a while ago, most ABC file properties could be abstracted with auto-implemented interfaces nowadays but it wasn't a thing at the time, and the lack of nullable support makes it kind of hard to deal with also. I was about to do it but this memory issue set me back, again it's really a shame that we cannot use span-based APIs for decompression from the BCL. Altough it seems that it have been implemented by a few people according to the issues you've mentionned, i'll check it when i'll have the time and maybe try to implement it |
Going to be pushing the current dev branch to nuget as 0.1.0 soon, so when these changes are introduced we can bump the major and release as 1.0.0. |
Gonna re-do this spanification is "smaller" pull requests. |
wip mess. way too big PR but seems to be the only way when you're touching every single line of the project, sad.