ClojureScript

Track bound dynamic variables to support binding in async mechanisms.

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: 1.7.228
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Environment:
    Any cljs version.

Description

The issue has been raised before:

While the reasoning behind the proposal is still valid, the original approach has made no progress due to the performance penalty. I have implemented a simplified approach with mutable JavaScript datastructures to minimize the performance impact. Because we are single-threaded we can use js assignment and don't need to port Clojure's binding frame. A small penalty is paid by the user of binding (see benchmark8) and a higher one by async mechanisms capturing and restoring the bindings (benchmark1-7):

https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a

I would provide patches to ClojureScript, if this looks like a worthwhile approach.

Activity

Hide
Antonin Hildebrand added a comment - - edited

Just for record I commented on it here: https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a#gistcomment-1764489. Not sure if GitHub sends out notifications about new gist comments.

Show
Antonin Hildebrand added a comment - - edited Just for record I commented on it here: https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a#gistcomment-1764489. Not sure if GitHub sends out notifications about new gist comments.
Hide
Christian Weilbach added a comment -

Thanks for pointing it out. David Nolen has also pointed out prototype chains to address this issue and now I see what he meant. I am not familiar enough with the internals of "this" in JavaScript, but one problem I see is that you need to distinguish dynamic vars at the call site. The advantage of using an object directly and capturing and restoring the frame explicitly when you enter and leave the code is that call sites are totally unaffected. The cost is only paid a little at the binding site and mostly in async libraries (bound-fn). But I might still need to look further into "this" . I have not got the gist comment from github.

Show
Christian Weilbach added a comment - Thanks for pointing it out. David Nolen has also pointed out prototype chains to address this issue and now I see what he meant. I am not familiar enough with the internals of "this" in JavaScript, but one problem I see is that you need to distinguish dynamic vars at the call site. The advantage of using an object directly and capturing and restoring the frame explicitly when you enter and leave the code is that call sites are totally unaffected. The cost is only paid a little at the binding site and mostly in async libraries (bound-fn). But I might still need to look further into "this" . I have not got the gist comment from github.
Hide
Antonin Hildebrand added a comment - - edited

Correct.

> you need to distinguish dynamic vars at the call site

I agree. My initial motivation was to solve a bit different problem without cooperation from library authors. I didn't want to modify ClojureScript behaviour and wanted to be just touching own code or doing trivial changes in library forks. Just wanted to share my thoughts about the implementation.

I have a feeling that solving this "async context" problem will be difficult. You will need async library authors to adapt their libraries. And users unaware of this will be running into issues anytime they step outside of bound-fn aware async libraries (for example using raw js interop). I believe Angular people solved this robustly in https://github.com/angular/zone.js. The implementation is quite scary monkey-patching, but if they were able to wrap all existing async calls at lowest level, maybe we could just build on top of their foundation and use zone.js as parallel mechanism for `binding`.

Show
Antonin Hildebrand added a comment - - edited Correct. > you need to distinguish dynamic vars at the call site I agree. My initial motivation was to solve a bit different problem without cooperation from library authors. I didn't want to modify ClojureScript behaviour and wanted to be just touching own code or doing trivial changes in library forks. Just wanted to share my thoughts about the implementation. I have a feeling that solving this "async context" problem will be difficult. You will need async library authors to adapt their libraries. And users unaware of this will be running into issues anytime they step outside of bound-fn aware async libraries (for example using raw js interop). I believe Angular people solved this robustly in https://github.com/angular/zone.js. The implementation is quite scary monkey-patching, but if they were able to wrap all existing async calls at lowest level, maybe we could just build on top of their foundation and use zone.js as parallel mechanism for `binding`.
Hide
Christian Weilbach added a comment - - edited

The angle I am coming from is roughly described here: https://github.com/fullcontact/full.monty/pull/9#issuecomment-131152058

I only found out at the very end when I had supervision of go-channels completely implemented, that the cljs binding was not behaving like the Clojure one. Arguments pro/contra binding in Clojure are also referenced. The zone monkey patching looks very heavy and prone to cause hairy bugs. It is noteworthy that Clojure does not embrace bindings, but keeps them always thread-local. So there you also have to use bound-fn or something similar whenever code is executed concurrently. core.async for instance uses the Clojure mechanism to push (capture) and pop (restore) bindings. I would like to have this in ClojureScript as well. I think one should not retain all bindings automatically, but rather allow the library author to handle dynamic bindings. I only track the supervisor binding for instance. For ClojureScript as for Clojure libraries and wrappers this should be fine. Pure JavaScript libraries usually have their own binding concepts like zone.js, right?

Tracking bindings is neither for free in Clojure nor in ClojureScript and it is an important design goal to embrace the host. In fact originally I tried to capture and restore all bindings. My benchmarks for tracking more dynamic vars (instead of just the currently active binding), were linearly more expansive than rebinding fewer selected vars and become prohibitive when you reach a few hundred.

Show
Christian Weilbach added a comment - - edited The angle I am coming from is roughly described here: https://github.com/fullcontact/full.monty/pull/9#issuecomment-131152058 I only found out at the very end when I had supervision of go-channels completely implemented, that the cljs binding was not behaving like the Clojure one. Arguments pro/contra binding in Clojure are also referenced. The zone monkey patching looks very heavy and prone to cause hairy bugs. It is noteworthy that Clojure does not embrace bindings, but keeps them always thread-local. So there you also have to use bound-fn or something similar whenever code is executed concurrently. core.async for instance uses the Clojure mechanism to push (capture) and pop (restore) bindings. I would like to have this in ClojureScript as well. I think one should not retain all bindings automatically, but rather allow the library author to handle dynamic bindings. I only track the supervisor binding for instance. For ClojureScript as for Clojure libraries and wrappers this should be fine. Pure JavaScript libraries usually have their own binding concepts like zone.js, right? Tracking bindings is neither for free in Clojure nor in ClojureScript and it is an important design goal to embrace the host. In fact originally I tried to capture and restore all bindings. My benchmarks for tracking more dynamic vars (instead of just the currently active binding), were linearly more expansive than rebinding fewer selected vars and become prohibitive when you reach a few hundred.
Hide
Antonin Hildebrand added a comment - - edited

I would be happy if your proposal went through. It would help my use-cases as well.

I'm going to explore zone.js when I get some spare time. I will try do write a wrapper library and implement an alternative mechanism to bindings using zone.js. I would like to provide this functionality as a library without a need to modify ClojureScript compiler or involvement from library authors.

Show
Antonin Hildebrand added a comment - - edited I would be happy if your proposal went through. It would help my use-cases as well. I'm going to explore zone.js when I get some spare time. I will try do write a wrapper library and implement an alternative mechanism to bindings using zone.js. I would like to provide this functionality as a library without a need to modify ClojureScript compiler or involvement from library authors.
Hide
Christian Weilbach added a comment -

Ok, I am curious how well this will work. Would this work with the state-machine transformation of core.async?

Show
Christian Weilbach added a comment - Ok, I am curious how well this will work. Would this work with the state-machine transformation of core.async?
Hide
Antonin Hildebrand added a comment -

I believe so. Core.async state machine uses only setTimeout and goog.async.nextTick. We can teach zone.js to deal with nextTick by setting goog.async.nextTick.wrapCallback_ with zone wrapping. Also if user decided to use any async API in their go blocks it should work, because zone.js will carry proper zone binding over async boundaries.

Show
Antonin Hildebrand added a comment - I believe so. Core.async state machine uses only setTimeout and goog.async.nextTick. We can teach zone.js to deal with nextTick by setting goog.async.nextTick.wrapCallback_ with zone wrapping. Also if user decided to use any async API in their go blocks it should work, because zone.js will carry proper zone binding over async boundaries.
Hide
Antonin Hildebrand added a comment -

I have opened this issue in zone.js: https://github.com/angular/zone.js/issues/342

Show
Antonin Hildebrand added a comment - I have opened this issue in zone.js: https://github.com/angular/zone.js/issues/342
Hide
Christian Weilbach added a comment -

Hey. Have you made any progress with implementing a small cljs demo with zone.js yet?

Show
Christian Weilbach added a comment - Hey. Have you made any progress with implementing a small cljs demo with zone.js yet?
Hide
Antonin Hildebrand added a comment -

Hi Christian. No, unfortunately I didn't get to it.

Show
Antonin Hildebrand added a comment - Hi Christian. No, unfortunately I didn't get to it.
Hide
Christian Weilbach added a comment -

Interestingly to implement the Common Lisp like condition system chouser presented here https://www.youtube.com/watch?v=zp0OEDcAro0 the dynamic binding working over async boundaries is also important.

Show
Christian Weilbach added a comment - Interestingly to implement the Common Lisp like condition system chouser presented here https://www.youtube.com/watch?v=zp0OEDcAro0 the dynamic binding working over async boundaries is also important.
Hide
Antonin Hildebrand added a comment -

Christian, please have a look at my implementation:
https://github.com/binaryage/cljs-zones

I have implemented the prototype trick as a library. It is just a gist of the idea, didn't spend time to make it robust yet and ES3-compatible. Re-binding frames should be as cheap as changing pointers (inside JS runtime).

Show
Antonin Hildebrand added a comment - Christian, please have a look at my implementation: https://github.com/binaryage/cljs-zones I have implemented the prototype trick as a library. It is just a gist of the idea, didn't spend time to make it robust yet and ES3-compatible. Re-binding frames should be as cheap as changing pointers (inside JS runtime).
Hide
Christian Weilbach added a comment -

Very nice work! I am checking it out atm. Nice that it is self-contained. (The Klipse version throws a goog.object not found error for me btw.)

Show
Christian Weilbach added a comment - Very nice work! I am checking it out atm. Nice that it is self-contained. (The Klipse version throws a goog.object not found error for me btw.)
Hide
Christian Weilbach added a comment -

I have updated the gist to incorporate cljs-zones in benchmark 8, 9 and 10. It is a bit faster in restoring dynamic bindings, but not much:

https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a

But there is a significant performance penalty on var access:

full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 1 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs
null
full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 2 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 6 msecs
null
full.binding_test.benchmark10()
core.cljs:150 [], ((fn [] v1)), 10000 runs, 3 msecs
core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs

Is this penalty addressable?

One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism, but this might be acceptable.

Show
Christian Weilbach added a comment - I have updated the gist to incorporate cljs-zones in benchmark 8, 9 and 10. It is a bit faster in restoring dynamic bindings, but not much: https://gist.github.com/whilo/a8ef2cd3f0e033d3973880a2001be32a But there is a significant performance penalty on var access: full.binding_test.benchmark10() core.cljs:150 [], ((fn [] v1)), 10000 runs, 1 msecs core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs null full.binding_test.benchmark10() core.cljs:150 [], ((fn [] v1)), 10000 runs, 2 msecs core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 6 msecs null full.binding_test.benchmark10() core.cljs:150 [], ((fn [] v1)), 10000 runs, 3 msecs core.cljs:150 [], ((fn [] (zones/get v1))), 10000 runs, 4 msecs Is this penalty addressable? One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism, but this might be acceptable.
Hide
Antonin Hildebrand added a comment -

Thanks for measuring it. I didn't really get to benchmarking yet.

Did you run the benchmarks under :advanced optimizations?

zones/get currently emits a call to goog.object/get, I'm not sure if this gets inlined by closure compiler, but if not, we can probably improve it by generating raw js object access:
https://github.com/binaryage/cljs-zones/blob/fdfa1421c39d64c9c6b9efbff474b7677f908197/src/lib/zones/core.clj#L94

> One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism
Can you elaborate? I don't understand how it would interfere.

Show
Antonin Hildebrand added a comment - Thanks for measuring it. I didn't really get to benchmarking yet. Did you run the benchmarks under :advanced optimizations? zones/get currently emits a call to goog.object/get, I'm not sure if this gets inlined by closure compiler, but if not, we can probably improve it by generating raw js object access: https://github.com/binaryage/cljs-zones/blob/fdfa1421c39d64c9c6b9efbff474b7677f908197/src/lib/zones/core.clj#L94 > One other issue is that JavaScript prototype manipulations through "this" would interfere with the cljs binding mechanism Can you elaborate? I don't understand how it would interfere.
Hide
Christian Weilbach added a comment -

With advanced compilation I get:

full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 2 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 3 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs
null
full.binding_test.benchmark10()
client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs
client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 1 msecs

But this is a very simple test. If you see a better way to benchmark it, go ahead . When the chains get prototype chains get longer, it might be more painful to walk the chain for each global lookup.

> Can you elaborate? I don't understand how it would interfere.
There has been a proposal somewhere to pass "this" in all cljs functions instead of null as first argument. Your explicit model of a zone is pretty safe I think, but if people would interefere with prototypes in JS, then this might break the "this" approach in subtle ways. Without passing "this" as a direct argument and doing it in a more JS way, you have to setup the zones all the time instead of using the scope chain, which might also contribute to the managing cost. So the "this" approach might help with performance, but I am not sure. You still have the chain overhead. But I am no JS expert, you know much better than me what you are doing.

Show
Christian Weilbach added a comment - With advanced compilation I get: full.binding_test.benchmark10() client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs null full.binding_test.benchmark10() client.js:278 [], ((fn [] v1)), 10000 runs, 2 msecs client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs null full.binding_test.benchmark10() client.js:278 [], ((fn [] v1)), 10000 runs, 3 msecs client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs null full.binding_test.benchmark10() client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 2 msecs null full.binding_test.benchmark10() client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 3 msecs null full.binding_test.benchmark10() client.js:278 [], ((fn [] v1)), 10000 runs, 1 msecs client.js:278 [], ((fn [] (zones/get v1))), 10000 runs, 1 msecs But this is a very simple test. If you see a better way to benchmark it, go ahead . When the chains get prototype chains get longer, it might be more painful to walk the chain for each global lookup. > Can you elaborate? I don't understand how it would interfere. There has been a proposal somewhere to pass "this" in all cljs functions instead of null as first argument. Your explicit model of a zone is pretty safe I think, but if people would interefere with prototypes in JS, then this might break the "this" approach in subtle ways. Without passing "this" as a direct argument and doing it in a more JS way, you have to setup the zones all the time instead of using the scope chain, which might also contribute to the managing cost. So the "this" approach might help with performance, but I am not sure. You still have the chain overhead. But I am no JS expert, you know much better than me what you are doing.
Hide
Christian Weilbach added a comment -

My last comment was a little bit confusing. I only see the problem with an impact on dereferencing vars. In the benchmark above the prototype chain is very short (v1 can be directly resolved) and already has a significant penalty, but I think the penalty gets even worse for not rebound root vars if you have a higher nesting level for the binding and the chain gets longer. Can you address this somehow?

Show
Christian Weilbach added a comment - My last comment was a little bit confusing. I only see the problem with an impact on dereferencing vars. In the benchmark above the prototype chain is very short (v1 can be directly resolved) and already has a significant penalty, but I think the penalty gets even worse for not rebound root vars if you have a higher nesting level for the binding and the chain gets longer. Can you address this somehow?
Hide
David Nolen added a comment -

We're just not interested in pursuing this at this time. If at some future point Clojure itself decides to something about this we'll consider it.

Show
David Nolen added a comment - We're just not interested in pursuing this at this time. If at some future point Clojure itself decides to something about this we'll consider it.

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: