Closed 88kami88 closed 7 years ago
@jvivs Check this out.
@divmain
Something's not right in that benchmark, how can the second pass with cached components take longer than the first pass of caching?
I'm going to want to take a look at this more closely. Those benchmarks indicate a significant performance regression. There might be a better-performing solution to the issues raised here.
My first guess would be overhead from naive promise implementation, specifically waiting for all segments to resolve in series, even when there's not an async cache get.
I'm not sure how much you should index to the benchmark when async caching doesn't seem to be working properly in the first place. ex: https://github.com/Rymar/rapscalion-cache is the exact problem I'm finding when trying to do async caching in a production application
Honestly the benchmarks 2.1.7
don't look much different:
renderToString took 11.486885729 seconds
rapscallion, no caching took 43.693205031 seconds; ~0.26x faster
rapscallion, caching DIVs took 16.163418342 seconds; ~0.71x faster
rapscallion, caching DIVs (second time) took 14.280983280 seconds; ~0.8x faster
rapscallion, caching Components took 0.701809205 seconds; ~16.36x faster
rapscallion, caching Components (second time) took 15.668481997 seconds; ~0.73x faster
rapscallion (pre-rendered), no caching took 35.028802432 seconds; ~0.32x faster
rapscallion (pre-rendered), caching DIVs took 18.111153076 seconds; ~0.63x faster
rapscallion (pre-rendered), caching DIVs (second time) took 17.383266673 seconds; ~0.66x faster
rapscallion (pre-rendered), caching Components took 16.657128768 seconds; ~0.68x faster
rapscallion (pre-rendered), caching Components (second time) took 16.814332255 seconds; ~0.68x faster```
Now back on this cache-miss
branch, executed immediately afterwards with similar load on same machine:
renderToString took 11.203203946 seconds
rapscallion, no caching took 41.365911048 seconds; ~0.27x faster
rapscallion, caching DIVs took 15.757512965 seconds; ~0.71x faster
rapscallion, caching DIVs (second time) took 13.955430961 seconds; ~0.8x faster
rapscallion, caching Components took 0.529333810 seconds; ~21.16x faster
rapscallion, caching Components (second time) took 12.946540060 seconds; ~0.86x faster
rapscallion (pre-rendered), no caching took 25.377965259 seconds; ~0.44x faster
rapscallion (pre-rendered), caching DIVs took 13.801301708 seconds; ~0.81x faster
rapscallion (pre-rendered), caching DIVs (second time) took 13.337650601 seconds; ~0.83x faster
rapscallion (pre-rendered), caching Components took 13.305775981 seconds; ~0.84x faster
rapscallion (pre-rendered), caching Components (second time) took 12.985989510 seconds; ~0.86x faster
It might actually be faster, granted the smallest of sample sizes in a personal computing environment.
I pulled this down and you're right - the benchmarks don't look much difference between v2.1.7 and your branch, although it seems to perform much better against React on my machine.
@ethersage, can you clean up the merge conflicts? I'll follow up with publishing a new version.
I will look at merge conflicts
@fleg Please review where I've tagged you to make sure I merged correctly.
lgtm, what about tests?
Tests look good to me.
Alrighty, I think this is ready to ship. Thanks for your work on this!
When using an async cache strategy and there's a cache miss, segments were being rendered out of order or not at all. This is caused by the fact that the cache segment is returning a promise, but the sequence continued to allow the emission of segments before the cache had resolved.
This PR makes almost everything a promise and builds a chain of promises where the next item in the sequence must wait for the previous item to render.
@divmain @maxgalbu
Resolves #87
TODO: