Expose liblzma's 'preset_dict' feature

[anall]

DO NOT MERGE

This is a mostly hacked together [not even minimally viable, just enough to get it to work] implementation to see if this is something you are willing to consider adding to this library in this current manner. I will happily accept opinions if you believe you have a better way to accomplish this.

My current implementation has a couple of known shortcomings:

• On earlier versions of Perl that do not have SV/string copy-on-write semantics, the dictionary (which may be several megabytes) will be copied -- possibly pass it around as a scalar ref instead of the string itself?
• Obviously, I'm just using the string from the SV without claiming it in any way. I didn't want to spend time on a proper solution if this feature is not something wanted in this module.

Thank you.

[pmqs]

Hey @anall thanks for the update. Yes, I'm interested in integrating the change. Will look at the your code but I'm really short on time available to work on it. If you do have time to make it more bullet-proof, that would really help.

[anall]

This additional commit should be safer and more bullet-proof but did require a more extensive change to the codebase.

I am not sure if I am happy that Perl will copy the preset dictionary, but I am at the limit of my XS knowledge and am unsure of the best way to prevent that copy without exposing other problems.

My only thought, especially for preventing copies even on earlier versions of Perl, is still to somehow "steal" the buffer out of the provided SV.

Support for "copy on write"/CoW scalars would also be useful, but that is beyond my XS knowledge.

But that is more of a performance concern and might be a case of premature optimization.

One other outstanding question: given the amount of duplicated code and similarity, should I get rid of Lzma::Filter::Lzma::PresetDict and instead add the PresetDict as an option to Lzma::Filter::Lzma?

[pmqs]

Found a spare moment to a preliminary scan the changes. All is looking good so far. Even better, you've added tests! Only thing I see missing are a few paragraphs for the documentation.

Wouldn't worry about duplicated code or COW at this point.

Now I just need to find some time the check the code in mode detail.

[pmqs]

Regarding some of your questions/comments

This additional commit should be safer and more bullet-proof but did require a more extensive change to the codebase.

I am not sure if I am happy that Perl will copy the preset dictionary, but I am at the limit of my XS knowledge and am unsure of the best way to prevent that copy without exposing other problems.

My only thought, especially for preventing copies even on earlier versions of Perl, is still to somehow "steal" the buffer out of the provided SV.

Support for "copy on write"/CoW scalars would also be useful, but that is beyond my XS knowledge.

I've never had to use COW in any of the XS code I've written.

But that is more of a performance concern and might be a case of premature optimization.

I agree that taking a copy of the preset dictionary isn't optimal, but will do for now.

One possibility is to pass a reference to the preset dict into the XS code (or create the reference with newRV_inc in the XS code) and store that in the XS object. That will prevent the dictionary being destroyed until the XS DESTROY method explicitly deletes it. The main issue with that approach is you can't prevent the contents of the preset dict being modified by code that runs after the creation of the filter.

FYI, the zlib equivalent of preset dictionaries is much more straightforward. You just call deflateSetDictionary (see zlib) with the dictionary once. Under the hood zlib does whatever processing that is needed, so there is no need to store the preset dict in an XS object.

One other outstanding question: given the amount of duplicated code and similarity, should I get rid of Lzma::Filter::Lzma::PresetDict and instead add the PresetDict as an option to Lzma::Filter::Lzma?

Yes, thought about this some more and I agree. Having a duplicate of Lzma::Filter::Lzma that just adds the PresetDict option just adds more complexity to an already complex interface.

[anall]

The main issue with that approach is you can't prevent the contents of the preset dict being modified by code that runs after the creation of the filter.

Yeah, that was my main concern, especially if the modifications require Perl to reallocate the buffer.

The copy does seem to be the safest option.

I've gone and rebased my changes into a single commit, and I believe this is now ready.

[pmqs]

Thanks @anall, change merged.