Skip to content

Add map-padded#102

Open
NoahTheDuke wants to merge 2 commits intoweavejester:masterfrom
NoahTheDuke:nb/map-padded
Open

Add map-padded#102
NoahTheDuke wants to merge 2 commits intoweavejester:masterfrom
NoahTheDuke:nb/map-padded

Conversation

@NoahTheDuke
Copy link
Copy Markdown

A demonstration for #101.

@weavejester
Copy link
Copy Markdown
Owner

I don't think you need a volatile or anything as complex. We can take the normal map implementation, and replace first with #(if % (first %) val) where val is the padding value, and swap out every? for some:

(defn map-padded [f val c1 c2 & colls]
  (let [step (fn step [cs]
               (lazy-seq
                (let [ss (map seq cs)]
                  (when (some identity ss)
                    (cons (map #(if % (first %) val) ss)
                          (step (map rest ss)))))))]
     (map #(apply f %) (step (conj colls c2 c1)))))

Then we just need some more arities for performance in common cases. I've factored out the padded "first" function into an outer letfn:

(letfn [(first* [s v] (if s (first s) v))]
  (defn map-padded
    ([f val c1 c2]
     (lazy-seq
      (let [s1 (seq c1) s2 (seq c2)]
        (when (or s1 s2)
          (cons (f (first* s1 val) (first* s2 val))
                (map-padded f val (rest s1) (rest s2)))))))
    ([f val c1 c2 c3]
     (lazy-seq
      (let [s1 (seq c1) s2 (seq c2) s3 (seq c3)]
        (when (or s1 s2 s3)
          (cons (f (first* s1 val) (first* s2 val) (first* s3 val))
                (map-padded f val (rest s1) (rest s2) (rest s3)))))))
    ([f val c1 c2 c3 & colls]
     (let [step (fn step [cs]
                  (lazy-seq
                   (let [ss (map seq cs)]
                     (when (some identity ss)
                       (cons (map #(first* % val) ss)
                             (step (map rest ss)))))))]
        (map #(apply f %) (step (conj colls c3 c2 c1)))))))

It looks like the multi-coll version of sequence is written using map, so we can presumably write sequence-padded with map-padded.

@NoahTheDuke
Copy link
Copy Markdown
Author

I used your implementation except for some small changes, which I left as comments.

@NoahTheDuke NoahTheDuke marked this pull request as ready for review March 27, 2026 17:27
@NoahTheDuke
Copy link
Copy Markdown
Author

NoahTheDuke commented Mar 27, 2026

I did some performance testing between my initial version and your new version (using criterium/quick-bench), and above 3 colls (when there's variable number of colls), yours was slower. With my changes (noted in the comments), ours are very similar, enough that I don't think further optimization is possible or worthwhile.

2 colls mine:
Evaluation count : 273924 in 6 samples of 45654 calls.
             Execution time mean : 2.295586 µs
2 colls yours:
Evaluation count : 293622 in 6 samples of 48937 calls.
             Execution time mean : 2.143859 µs

3 colls mine:
Evaluation count : 115620 in 6 samples of 19270 calls.
             Execution time mean : 5.377075 µs
3 colls yours:
Evaluation count : 118926 in 6 samples of 19821 calls.
             Execution time mean : 5.177104 µs

4 colls mine:
Evaluation count : 67104 in 6 samples of 11184 calls.
             Execution time mean : 9.096748 µs
4 colls yours initial:
Evaluation count : 31038 in 6 samples of 5173 calls.
             Execution time mean : 19.493132 µs
4 colls yours new:
Evaluation count : 59418 in 6 samples of 9903 calls.
             Execution time mean : 11.140960 µs

5 colls mine:
Evaluation count : 43548 in 6 samples of 7258 calls.
             Execution time mean : 13.982453 µs
5 colls yours initial:
Evaluation count : 19038 in 6 samples of 3173 calls.
             Execution time mean : 32.063496 µs
5 colls yours new:
Evaluation count : 39852 in 6 samples of 6642 calls.
             Execution time mean : 15.403038 µs

@NoahTheDuke
Copy link
Copy Markdown
Author

As I said in the issue, sequence is pretty complicated (TransformerIterator, and RT/chunkIteratorSeq rely a lot on details i don't fully understand), so I can't say much about how we'd implement it.

@weavejester
Copy link
Copy Markdown
Owner

Thanks for doing the benchmarks, and for the performance improvements.

I initially thought sequence-padded would be straightforward - replacing the map with map-padded - but I see that it pivots the collections and creates an iterator for each. I'll take a closer look at what would be required.

@NoahTheDuke
Copy link
Copy Markdown
Author

I found a tidy solution for sequence-padded implemented in Clojure, inspired by this blog post. I didn't do any performance testing but relied on the existing techniques to keep it overall performant.

@NoahTheDuke
Copy link
Copy Markdown
Author

looking at the code again, i should write a test verifying reduced works in both cases, and a test checking that a more complex xform works for sequence-padded.

(lazy-seq
(let [ss (mapv seq cs)]
(if (some identity ss)
(let [res (apply f nil (mapv #(first* % val) ss))]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not wrap both functions in the letfn. Instead, we can inline first*.

(mapv #(if % (first %) val) ss)

It might also be worth doing that for map-padded... on reflection, the first* function doesn't save much space...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first* is used a ton in map-padded because we have 3 arities. It's only used once in sequence-padded, so I think removing it from that probably makes the most sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants