Speed up js->clj on objs using forEach and transient volatile
Description
Environment
Attachments
Activity
Mike Fikes May 12, 2019 at 2:35 AM
CLJS-2341-2.patch added to Patch Tender
Mike Fikes June 30, 2018 at 4:12 PM
Since CLJS-844 has been applied, the previous patch and benchmarks are no longer applicable. Attaching re-basedlined patch. It gives mixed results relative to the code now on master. Including updated benchmarks here so that a decision can be taken on whether it is worth applying or not.
Thomas Mulvaney April 30, 2018 at 5:00 PM
Francis is right, ignore me. Bashing things in place that don't change type works but possibly might break if implementations change. Docs clearly say its a bad idea - just because I've seen it done doesn't make it ok!
Francis Avila April 30, 2018 at 4:35 PM
No, wrong TLDR: in this particular patch you cannot bash (transient {}) in place, because this starts out as an array-map and returns a transient hash-map once assoc! would return a map with a count greater than 8.
PersistentHashMap.createWithCheck (the example Thomas cited) can bash because it is written very carefully and knows all the types and their implementations.
If you only use hash-maps e.g. with (transient (hash-map)) you can bash in place, but then you won't have array-map for small maps.
(Really, don't bash transients in place.)
Mike Fikes April 30, 2018 at 4:13 PM
So, TL;DR is that, with some risk of brittleness, we could bash in place and potentially get more perf.
Details
Assignee
David NolenDavid NolenReporter
Mike FikesMike FikesLabels
Patch
CodeAffects versions
Priority
Minor
Details
Details
Assignee
David NolenReporter
Mike FikesLabels
Patch
Affects versions
Priority
Minor
It is possible to speed up
js->cljon JavaScript objects by revising the implementation to employgoog.object/forEach, accumulating by bashing on a transient volatile map.The speedups range from 1.5 to 2.1 over the current implementation.
Note: The current implementation uses
js-keys. The optimization in could helpjs->clj, but it doesn't appear to provide much speedup in itself (perhaps 1.1?) compared to the change in implementation described above.