[WIP] transformation-redux

[ZoeyR]

This is a heavy WIP PR to redo the underlying transformation system. Currently the branch has test failure for the flif_logo, some features were regressed and performance hasn't improved. I'll be evolving this PR in public to hopefully get some useful feedback.

[ZoeyR]

updated the code to get rid of dynamic dispatch and restore the original API, but now there are more failures and it actually performs worse :/. I suspect I messed up YCoCg somehow.

[ZoeyR]

I've spent some time optimizing this and I still can't get it faster than the dynamic dispatch implementation of transformations. @newpavlov you're better at optimization than me is there anything obvious I've missed?

[newpavlov]

I have several minor ideas how code can be improved, but not sure why you get a slowdown. It will be hard to tell without playing with the code. My current suggestions are:

• It will be nice to measure performance of the TransformationSet methods directly in the module level benchmark. (i.e. build it based on real images and then benchmark methods directly)
• You probably don't need Transform trait any more.
• Orig transform probably can be removed
• Some trivial methods can be written directly into match arms
• Not sure why you need last field in the TransformationSet, can't you just use split_last?
• Play with inlining here and there?
• We could remove transformations field from SecondHeader (thus making transformations info inaccessible to users), then it will be possible to make Transformation and TransofrmationSet generic over Pixel, which may simplify code a bit.

[ZoeyR]

I have the last field in the transformation set as a small optimization. We're guaranteed to have at least one transformation so it removes one bounds check.

[ZoeyR]

Making transformations general over pixel type has some issues. The first commit in this PR used that and ran into a problem: the last field of the second header is stored after the transformations, so we wouldn't be able to expose that to users.

[newpavlov]

Yes, I've wrote about it in my comment. IIUC currently you expose only transformation names and their order, without any internal information. If you think that it's worth to expose this information (personally I don't care about it, and AFAIK libflif does not expose this information either), then it should be possible to create a dummy list.

[ZoeyR]

It's probably worth it to evaluate what information needs to be exposed to the user. The reason I exposed transformation data is that I sorely missed having it available when trying to implement this decoder. That's probably a very specific use-case though so I can see why it wouldn't be necessary. Perhaps it's better just to put it into a debug log.