clj-commons / byte-streams

A Rosetta stone for JVM byte representations
417 stars 33 forks source link

Lazy converter instantiation performance gotcha #62

Open DerGuteMoritz opened 1 year ago

DerGuteMoritz commented 1 year ago

Converters are instantiated (and then memoized) lazily on first use. Since this is a rather heavy operation, it can influence timing-sensitive code as recently observed in Aleph's test suite. This is bit of a gotcha which was even brought up in a past issue already. That resulted in the introduction of a precache-conversions API. However, it later got removed again without further explanation. In any case, the fact that this issue came up again indicates that such an API would still be desirable.

KingMob commented 1 year ago

Before we start talking about an API, let's define the problem more clearly.

We know the late binding can mess with tests and other initial conversions.

We don't know the impact of just doing it eagerly. Based on the test, we know it's more than a few hundred ms, but that's not a big deal at startup time for a long-running process. Can you time the eager setup of all conversions? How bad is it really? If it's not too bad, we should just simplify and switch. It's totally possible the initial setup isn't as painful as when byte-streams was created.

If it's considerable, we do have another option. We could split the def-conversions into different namespaces, and users would require only the ones they need. Not sure that's much better, though.

In general, I'd prefer to optimize for predictability over startup time.