ClojureScript

possible protocol dispatch performance enhancement

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

We could at the call site do the bit test and call directly? Otherwise, call the slow path? Implications for code size, but perhaps this could be a flag for people who really want the performance?

Activity

Hide
David Nolen added a comment -

flag might be:

:inline-protocol-fns true
Show
David Nolen added a comment - flag might be:
:inline-protocol-fns true
Hide
Michał Marczyk added a comment - - edited

No switch in this version – will add one if it seems it's worth it.

With this applied, core-advanced-test.js grows in size to 360301 bytes from 357528 bytes on current master.

Show
Michał Marczyk added a comment - - edited No switch in this version – will add one if it seems it's worth it. With this applied, core-advanced-test.js grows in size to 360301 bytes from 357528 bytes on current master.
Hide
Michał Marczyk added a comment -

This does seem to be a perf win on both :simple and :advanced, by the way.

Show
Michał Marczyk added a comment - This does seem to be a perf win on both :simple and :advanced, by the way.
Hide
Michał Marczyk added a comment -

Reattaching the patch to CLJS-246. Will get back to this.

Show
Michał Marczyk added a comment - Reattaching the patch to CLJS-246. Will get back to this.
Hide
David Nolen added a comment -

After some discussion with Rich this is definitely a direction we should pursue. It's very similar to what Clojure currently does.

Show
David Nolen added a comment - After some discussion with Rich this is definitely a direction we should pursue. It's very similar to what Clojure currently does.
Hide
David Nolen added a comment - - edited

I went ahead and tried this, http://github.com/clojure/clojurescript/compare/master...cljs-247-protocol-fn-call-sites.

Code size is not very different 364k vs 373k for compiling all of the tests.

This is not going to be faster given our use of satisfies? I noticed that if we removed the satisfies? calls this is a performance win.

Problem: If a symbol passes a satisfies? it would be nice if we could tag it as satisfying the protocol and just emit the protocol dispatch directly. However this would not work for JavaScript primitives.

Show
David Nolen added a comment - - edited I went ahead and tried this, http://github.com/clojure/clojurescript/compare/master...cljs-247-protocol-fn-call-sites. Code size is not very different 364k vs 373k for compiling all of the tests. This is not going to be faster given our use of satisfies? I noticed that if we removed the satisfies? calls this is a performance win. Problem: If a symbol passes a satisfies? it would be nice if we could tag it as satisfying the protocol and just emit the protocol dispatch directly. However this would not work for JavaScript primitives.
Hide
David Nolen added a comment - - edited

Looks like I spoke a bit too soon There was a typo in the branch. The performance improvement between this and master is small for V8, but measurable for SpiderMonkey and JavaScriptCore.

Also, we could do the satisfies? optimization such that it works for JS natives with a simple gensymed mutable local around the callsite.

Show
David Nolen added a comment - - edited Looks like I spoke a bit too soon There was a typo in the branch. The performance improvement between this and master is small for V8, but measurable for SpiderMonkey and JavaScriptCore. Also, we could do the satisfies? optimization such that it works for JS natives with a simple gensymed mutable local around the callsite.
Hide
David Nolen added a comment - - edited

K for handling natives simply tagging the symbol is not enough. We need to set a runtime gensymed flag that denotes whether it's a fast path protocol call. We can then avoid any redundants tests in the true branch of satisfies? and emit code like the following:

fast-path-flag ? foo.cljs$lang$Protocol$bar$arity0$(x) : cljs.core.Protocol.bar[goog.typeOf(x)](x);
Show
David Nolen added a comment - - edited K for handling natives simply tagging the symbol is not enough. We need to set a runtime gensymed flag that denotes whether it's a fast path protocol call. We can then avoid any redundants tests in the true branch of satisfies? and emit code like the following:
fast-path-flag ? foo.cljs$lang$Protocol$bar$arity0$(x) : cljs.core.Protocol.bar[goog.typeOf(x)](x);
Hide
David Nolen added a comment -

Closing for now, I think ^not-native type hint is probably the way to go if we want to avoid protocol dispatch overhead.

Show
David Nolen added a comment - Closing for now, I think ^not-native type hint is probably the way to go if we want to avoid protocol dispatch overhead.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: