[#CLJS-247] possible protocol dispatch performance enhancement - Clojure JIRA

Details

Type: Enhancement
Status: Open
Priority: Major
Resolution: Unresolved
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

Ascending order - Click to sort in descending order

Hide

Permalink

David Nolen added a comment - 10/May/12 11:19 AM

flag might be:

:inline-protocol-fns true

Show

David Nolen added a comment - 10/May/12 11:19 AM flag might be:

:inline-protocol-fns true

Hide

Permalink

Michał Marczyk added a comment - 10/May/12 11:57 AM - 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 - 10/May/12 11:57 AM - 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

Permalink

Michał Marczyk added a comment - 10/May/12 12:31 PM

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

Show

Michał Marczyk added a comment - 10/May/12 12:31 PM This does seem to be a perf win on both :simple and :advanced, by the way.

Hide

Permalink

Michał Marczyk added a comment - 10/May/12 1:09 PM

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

Show

Michał Marczyk added a comment - 10/May/12 1:09 PM Reattaching the patch to CLJS-246. Will get back to this.

Hide

Permalink

David Nolen added a comment - 17/May/12 1:33 PM

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 - 17/May/12 1:33 PM After some discussion with Rich this is definitely a direction we should pursue. It's very similar to what Clojure currently does.

Hide

Permalink

David Nolen added a comment - 17/May/12 9:29 PM - 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 - 17/May/12 9:29 PM - 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

Permalink

David Nolen added a comment - 18/May/12 12:40 AM - 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 - 18/May/12 12:40 AM - 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

Permalink

David Nolen added a comment - 23/May/12 9:56 AM - 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 - 23/May/12 9:56 AM - 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);

People

Assignee:

Michał Marczyk

Reporter:

David Nolen

Vote (0)

Watch (0)

Dates

Created:

10/May/12 11:10 AM

Updated:

23/May/12 9:57 AM

ClojureScript

possible protocol dispatch performance enhancement

Details

Description

Activity

People

Dates