manageplaces / Stompex

A STOMP client for Elixir
MIT License
12 stars 7 forks source link

Improve decompression feature. #10

Open shymega opened 5 years ago

shymega commented 5 years ago

Currently users of Stompex have the library decompress STOMP frames indiscriminately, without checking the STOMP frame for compression. Additionally, as we use the Erlang zlib module, it doesn't always work on all compression algos.

I'd like to introduce automatic decompression based on the signature of the compressed STOMP message frame.

Decompression should be enabled by default, and disabled based on an opt-out process.

One thing to consider: overhead. Will automatic decompression make performance suffer?

pareeohnos commented 5 years ago

I like the idea, but I think trying to do this automatically for every frame would introduce a pretty major overhead (though would need to benchmark to test). I think having it be able to handle different compression algo's would be good, though I'd be inclined to keep it as something configurable. I can't think of a use case where the server would randomly change algorithms, or send some compressed and others uncompressed.

shymega commented 5 years ago

It would, agreed. This might be better off being configurable, yes - in terms of use-cases, National Rail's NRDP platform now pushes both gzipped XML payloads, and in plain text status messages, albeit on different queues.. but worth noting!

pareeohnos commented 5 years ago

If they're in separate Queue's then this would be fine - it's up to the developers which queue they're using, and therefore whether they tell Stompex to try and decompress it or not. So I'd go with making this configurable, and then we can support different types of compression, though not sure how best to assess which ones would need supporting. Probably just add them as requested

shymega commented 5 years ago

Fair enough. Let's settle on configuration-based autodetection of compression algorithms and assessments on which ones need supporting. What would be even better, if one doesn't exist already, is to shove the compression algorithm detection code into a separate library and use that from Stompex?

pareeohnos commented 5 years ago

Sorry, I meant configuration based enabling of decompression full stop, rather than autodetecting. I think it would be better to be able to just say "decompress with this algorithm" - though I guess we could add another config option, so we'd have:

But I think the autodetect would only be good, if it autodetects on the first frame and then once it's done, it stops trying to autodetect and just uses the one it determined. This would remove the overhead of trying to detect every time

shymega commented 5 years ago

I see. Configuration proposal sounds suitable. Agreed about autodetecting on the first frame. Would have to store the suitable algorithm into the process state..