ClojureScript

Small PVs are never chunked

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.9.908
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code

Description

A PersistenVector that has <=32 elements and is seq'ed will return an IndexedSeq. Though IndexedSeq always fails the chunked-seq? check.

This means a:

(def xv (vec (range 32)))
(reduce + (map inc xv))

is about 4x slower than a map over a vector with 33 elements.

Options:
1. Return a ChunkedCons with the "rest" set to nil in PersistentVector.seq()
2. Implement IChunkedSeq for IndexedSeq:

(extend-type IndexedSeq
    IChunkedSeq
    (-chunked-first [x] x)
    (-chunked-rest [x] ())
    IChunkedNext
    (-chunked-next [x] nil)
    IChunk
    (-drop-first [coll]
      (if-some [n (-next coll)]
        n
        (throw (js/Error. "-drop-first of empty chunk")))))

I think option #2 is better since IndexedSeq is used quite a bunch throughout the code base, so the chunking will also kick in for many other code paths.

Activity

Hide
A. R added a comment -

Note:

This is different from Clojure (which does not consider an IndexedSeq a ChunkedSeq) since we use IndexedSeq a lot more where Clojure often uses a ChunkedSeq. For instance:

(apply (fn [& a] (type a)) [1 2 3 4 8])

will return IndexedSeq in CLJS but ChunkedCons in CLJ.

Since these IndexedSeq's will be passed around wildly in a normal CLJS app it makes sense to extend them as a IChunkedSeq since otherwise all these will never be chunked and get a slow first/next treatment.

Show
A. R added a comment - Note: This is different from Clojure (which does not consider an IndexedSeq a ChunkedSeq) since we use IndexedSeq a lot more where Clojure often uses a ChunkedSeq. For instance:
(apply (fn [& a] (type a)) [1 2 3 4 8])
will return IndexedSeq in CLJS but ChunkedCons in CLJ. Since these IndexedSeq's will be passed around wildly in a normal CLJS app it makes sense to extend them as a IChunkedSeq since otherwise all these will never be chunked and get a slow first/next treatment.
Hide
Mike Fikes added a comment -
Show
Mike Fikes added a comment - See CLJS-2926
Hide
Mike Fikes added a comment - - edited

CLJS-2467.patch passes in CI and Canary.

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

CLJS-2467.patch added to Patch Tender

Show
Mike Fikes added a comment - CLJS-2467.patch added to Patch Tender

People

Vote (6)
Watch (2)

Dates

  • Created:
    Updated: