search

haxetink / tink_core

Core utilities
https://haxetink.github.io/tink_core
MIT License
117 stars 33 forks source link

Non-lazy sync futures without extra allocation #132

Closed nadako closed 4 years ago

nadako commented 4 years ago

I couldn't sleep and I remembered one thing that was bothering me about tink futures: Currently Future.sync(42) will generate new SyncFuture(new LazyConst(42)), which is not a huge issue, because sync futures are not that frequent in general, but still a bit unfortunate.

So I added some abstract magic to overload the sync constructor to instantiate a yet another underlying Future implementation when the value is not Lazy<T>: SyncFutureConst, which is both FutureObject and LazyObject (the latter is needed for FutureStatus.Ready).

The current state is a bit dirty wrt naming and the fact that I had to un-private Lazy interfaces, but I'm curious about your opinion on the general approach here and whether you think it's worth it at all (I guess that depends on the target allocator/gc).

back2dos commented 4 years ago

Hmm, yeah, dunno. When it comes to memory, I'm surely way less defensive than the average game developer, so this type of stuff doesn't keep me up at night ^^

If possible, I would prefer not to make LazyObject public, because that means we can't make guarantees about the behavior of Lazy anymore. Perhaps this can be achieved by tweaking Haxe's access control though?

In the end, this is a composition-vs-inheritance thing, where I always prefer composition, unless it impacts performance. In this case, I'm skeptical that it does:

  1. Typically, the payload of a future will outsize a LazyConst by at least one order of magnitude, so the effect of this optimization is unlikely to be measurable.
  2. The case that's being optimized for assumes purely constant futures in such massive amounts that wrapping payload in Lazy would cause performance degradation. If so, then feeding the code with futures that actually are lazy/async would also degrade performance and so the whole abstraction breaks down anyway and was a poor fit for the problem from the start.

But yeah, I'm not against it, so long as it can be done without exposing internal interfaces.

back2dos commented 4 years ago

Imma close this for now. Yesterday I spent a few hours flattening some composition into inheritance (subclassing callbacklist instead of composing it) and much to my surprise I found that there was no speed increase on JS and on Java it even double runtime and quadrupled memory usage. So if you see an optimization opportunity, please accompany it with benchmarks ;)