ClojureScript

Have IndexedSeq implement IChunkedSeq

Details

  • Type: Enhancement Enhancement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Duplicate
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test

Description

In Clojure, small vectors support chunked-seq: For example (chunked-seq? (seq [1 2 3])).

In ClojureScript, this is currently not true because creating a ChunkedSeq instance is more expensive than simply creating an IndexedSeq instance for small vectors. See https://github.com/clojure/clojurescript/commit/e0fd7a07f959d6472a9b0c73b55be2627c40a0ec where IndexedSeq is created for vectors of size 31 or smaller.

But, if we consider making IndexedSeq itself implement IChunkedSeq we could regain perf for short vectors without making seq slower:

An experiment doing so exhibits these speedups under advanced

V8: 2.6, SpiderMonkey 2.4, JavaScriptCore 1.9, Nashorn 1.3, ChakraCore 2.2, GraalVM 2.8

for this case of a filter and map chain:

(simple-benchmark [xs [1 2 3 4 5 6 7 8 9 10 11 12 13 14]] (doall (filter even? (map inc xs))) 1e6)

Activity

Hide
Mike Fikes added a comment -

CLJS-2926.patch passes in CI and Canary tests.

Show
Mike Fikes added a comment - CLJS-2926.patch passes in CI and Canary tests.
Hide
Mike Fikes added a comment - - edited

See CLJS-2467. The advantages I see with the patch in CLJS-2467 (over the patch in this ticket are):

  1. It is probably faster as no arithmetic is done
  2. It implements IChunk
  3. With it, reduce on IndexedSeq is probably correct in more places (because it ignores end), which this patch make make things worse with respect to that by using end

This ticket might be better in that it actually chunks things up. (Perhaps this matters if you have a very long IndexedSeq and you have a deep chain of operations consuming the seq, but this is just speculation.)

Show
Mike Fikes added a comment - - edited See CLJS-2467. The advantages I see with the patch in CLJS-2467 (over the patch in this ticket are):
  1. It is probably faster as no arithmetic is done
  2. It implements IChunk
  3. With it, reduce on IndexedSeq is probably correct in more places (because it ignores end), which this patch make make things worse with respect to that by using end
This ticket might be better in that it actually chunks things up. (Perhaps this matters if you have a very long IndexedSeq and you have a deep chain of operations consuming the seq, but this is just speculation.)
Hide
Mike Fikes added a comment -

Closing as duplicate of CLJS-2467.

Show
Mike Fikes added a comment - Closing as duplicate of CLJS-2467.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: