adamhaile / surplus

High performance JSX web views for S.js applications
638 stars 26 forks source link

Question: Any ability to use Surplus behind Getter/Setters? #27

Open ryansolid opened 6 years ago

ryansolid commented 6 years ago

Even though I recognize the performance hit I was trying surplus behind ES5 Getter/Setters and ES6 Proxies and I noticed certain dependencies would just get skipped. It looked like the compiled templates would be slightly different whether the parenthesis were there or not.

I'm probably doing something terribly bad to performance by trying this. I just come from a Knockout background (used in production for about 7 years, training new devs etc). It's been clear to me for quite some time that while fine grained change dependency tracking is the superior approach(not knockout specifically) the api/syntax and the need to keep track of nested mapping prevents it from ever gaining a ton of traction. MobX the other large library with this sort of approach is still typically used with React and trapped by the fact updates are still per component, making it always limited by React and hence slower than it.

It's my belief that while having explicit computational wrappers is a must here, in the whole scheme of larger projects it's much cleaner than tracing through layers of shouldComponentUpdate and other very procedural lifecycle functions. However having data look like anything but POJO's is unacceptable.

I was very happy to have discovered this project 6 months back as none of my approaches I've worked on the last couple years beat out the fastest virtual dom ones across the board but I knew it should be possible. While it's likely the overhead of both the Surplus syntax and the proxying is less performant than just putting the logic in the proxying method itself, I was curious to try it for comparison sake.

In any case this project has been an inspiration and I learn something every time I look at the source code.

adamhaile commented 6 years ago

Hi Ryan -- Former Knockout dev here too :).

Ugh, you've fallen victim to one of my least favorite bits of code in surplus. Yes, you're exactly right: signals hidden behind getters and setters can evade detection, and parens make a difference. S's dependency detection is fast, but not zero cost, so the surplus compiler has an optimization to only look for dependencies where it thinks it might find some. If it thinks the code has no apparent signals (logic here), it doesn't wrap it in a computation. If it's wrong, then either the DOM won't update, or the containing computation (if any) picks up the dependency, making for a larger update than strictly necessary.

Your report inspires me to look for a better solution here. I'm thinking of putting this optimization behind a compiler flag.

In the meantime, you can convince the compiler to bail out of this optimization by wrapping the expression in parens: <input value={(foo.bar)} /> instead of just <input value={foo.bar} />. Not pretty, but it'll work.

For what it's worth, I've written reactive apps using both getters / setters and parens, and I have a definite preference for parens in my own code. The two big pluses of getters / setters are cleaner syntax and easier interop with 3rd-party libraries, but the loses are that signals are no longer first-class and dependencies become very difficult to track. The syntax issue is real, and the fact that it hits new devs the hardest sucks, no argument there 😦 . On the other hand, once you're over that, getters / setters make reactive code harder to reason about, limiting what you can build. It's a pain now vs pain later tradeoff, but the pain now is a fixed cost while the pain later ramps exponentially. The other plus, interop, I've found to be smaller than expected, because it's rare that you're passing your own objects into 3rd party code without some kind of adapter layer.

But, yeah, the new devs issue is real. In my experience, getters / setters start to show their limitations beyond simple object X bound to view Y scenarios, but that's all that new devs are building, so they feel all the pain with none of the benefits.

I'm not at all averse to providing a couple utilities that allow working with getters / setters instead of thunks. My thought had been something like an s-pojo package with possibly just two methods, one that acts like mobx's extendShallowObservable() utility and one like mobx's observable.ref decorator. I'd be curious to hear what your explorations were in this area. When you talk about "the pain of nested mapping" are you referring to the ko.mapping utility? We ended up having to tear that out of just about every codebase we used it in. It's one of the reasons I don't want to provide something like mobx's infectious observability. -- Adam

ryansolid commented 6 years ago

Yeah I understand completely. I do see the value of the Signals being first class citizens especially from the mindset of like TC39 Observables, where being able to express the transformations as a stream is clean and declarative. It is also the action of getting that triggers the dependency detection so hiding that behind normal property accessors can cause additional unintentional dependencies when accessing nested signals since the developer might not be even conscious they are triggering it without the explicit parenthesis.

That being said I've been still experimenting with ES6 Proxies. Aliasing the subscribable interface behind a different getter like the property name with a trailing $ etc. Being able to use destructuring and spread operators is nice. The thing about Proxies is that you can defer wrapping the children properties until they are accessed. So you can wrap at fetch time instead of on set. So even if it's like MobX deep observability it doesn't automatically make everything observable just the things you access when they are first accessed.

The nested mapping issue I'm talking about is just the general case of every time you have an array of objects that you want to listen to changes on specific properties of those objects or any nested children of those etc. I haven't used the ko.mapping utility but we had a client side ORM integration that auto generated observables for all the properties that involved creating typed view models for each "model" that if someone wasn't careful would create unexpected chains along the relationships and cause huge performance issues.

After I gutted that it didn't exactly remove the need to wrap models. I tried 2 approaches. The first was to add a "use" custom binding that would take an observable and on updating, update the child bindings. Basically like the knockout "with" binding except it wouldn't remove and replace all the child nodes just update the bindings in place. By doing this I could map the data to observables in the view while rendering it. I might iterate over a list with a foreach binding then wrap each model in it's own View Model or special observable (that triggered on certain property changes) with the 'use' binding on a descendant. This worked great and I was pretty happy with pattern until I needed to interact with the mapped data directly. Like looking to things like item selection where it's not part of the data model but part of the view representation and needing like a select all mechanism. It is possible to always use computeds to resolve and keep 2 lists but it wasn't obvious to the developers.

The second was to extend fn onto observables to do memoized mapping and filtering etc very RXjs like. Now I was essentially back to mapping the data except it wasn't magically happening in the background. This is fine it just meant having to do this all over the place explicitly. It just generally comes with the territory with observables or signals or what not the need to do this mapping in some form or another. It's not that painful just that it seems to be necessary.

Anyway I am largely still just exploring what can be done with proxies. Generally I"m not wrapping other observable frameworks in them. I was just curious how big of a hit they were on performance and was a little surprised when I noticed the difference parenthesis made. I was just thinking perhaps I'd notice something that I hadn't seen before where through proxies I was doing things especially inefficiently. And if the performance was high enough entertain it as a possibility. In any case your optimization makes a ton of sense. And naked Surplus in the tests I had set up smashed everything else by a large margin.

From the perspective of proxies the specific utilities are less important and wrapping the variable in parenthesis will do the trick fine. I'm going to spend a bit more time looking at this making comparisons. I will let you know if anything interesting comes of it.

ryansolid commented 6 years ago

I'd been toying with the old Jeremy Ashkenas circles browser test (modified to calc start to start instead of just the update loop since most frameworks have some amount of async scheduling, and upped the circles to 300). Not particularly scientific or precise but I like that it highlights the benefits of fine grained change detection and the tight loop identified GC issues. It is like a more extreme Partial Update from the JS Framework Benchmark. I threw together a proxy wrapper on surplus and was disappointed it was similar in performance to my other proxy attempts. Which while faster than most VDom implementations couldn't touch naked Surplus in the same scenario.

I was updating the CSS when I noticed I actually was gotten by the compiler optimization a 2nd time, not realizing because the visuals worked. This time it actually caught the dependency but in an outer computation causing unnecessary recalcs slowing it down. Fixing that in Chrome the proxy wrapper ended up being much smaller of an overhead. Like if average loop for Naked Surplus was 10.3ms, then Proxy Surplus 11.8ms, my Proxy Method 16.5ms, React 16 17.4ms.

This suggests something on the scheduling (microtask queue) rather than the proxies are my performance bottleneck. It's likely a proxy approach wrapping Surplus is still very performant. I'm going to try it with JS Framework Benchmark next. It's possible in Chrome a proxy wrapped version would still land in the same zone as the fastest VDom implementations.

adamhaile commented 6 years ago

I have a benchmarking script in the S repo you might find useful. You can run it via npm run bench. It tests the cost of creating and updating various topologies. Output looks like this. The "NtoM" numbers refer to the topology: "2to1" means two data signals watched by a single computation, "1to2" means one data signal watched by two computations, etc. It does a hard GC just before the test starts and just before it ends, to try and capture GC costs.

It'd be interesting to see how your Proxy wrapper benches.

The timings aren't as repeatable as I'd like :/. Some of them see substantial, like 30%, variation between runs. I haven't figured out why that is -- maybe even random things like cache locality. It's useful for capturing large regressions though.