Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Unroll assoc and assoc! for small numbers of arguments

Description

Making assoc calls of a single kv is significantly faster than one assoc of multiple kvs:

Michiel Borkent did some code spelunking to look at frequency of use per key count:

assoc kv arity count

m2 corpus count

Corfield corpus count

1 key

20884

32258

2 keys

3036

4848

3 keys

1083

1286

4 keys

327

535

5 keys

126

354

6 keys

64

118

7 keys

33

100

Proposed:

  • Impl: assoc is defined early, redefine later with the unrollings and apply the docstring, meta etc to that point.

  • Unroll up to 6 kvs

Perf testing of patch, similar to .

The last column is a check for polymorphic calls, which shows similar gains to those seen in the monomorphic tests.

Patch:

CLJ-1656-v8

Environment

None

Attachments

14

Activity

Show:

Rich Hickey August 21, 2023 at 1:53 PM

it might be best to make this faster by making varargs faster rather than unrolling every similar fn

Michiel Borkent December 21, 2020 at 4:20 PM
Edited

To gather some statistics of how many k/v pairs people typically use with assoc, I made this script:

https://gist.github.com/borkdude/e6f0b12f9352f3375e5f3277d2aba6c9

The output from my entire m2 directory:

([1 20884] [2 3036] [3 1083] [4 327] [5 126] [6 64] [7 33] [10 23] [8 11] [9 6] [14 4] [0 1] [12 1] [16 1])

Stats from 's code base:

([1 32258] [2 4848] [3 1286] [4 535] [5 354] [6 118] [7 100] [9 32] [11 22] [12 14] [18 10] [8 8] [29 6] [16 6] [19 2] [10 2])

Alex Miller August 13, 2018 at 10:27 PM

This still needs assessment on frequency of different counts. Based on some very quick checking, passing two kvs is common enough to matter. Passing three is far less common and so on. But I'd love to see some rough idea of frequency from running some stuff. I think the current unrolling is too much (2-3 kvs is probably sufficient).

Michael Blume September 28, 2016 at 8:14 PM

Updated patch to apply to master

Alex Miller April 30, 2015 at 5:10 AM

The existing arglist is fine - it just overrides the generated one for doc purposes.

Did you try any of the RT.assocN() stuff?

I guess another question I have is whether people actually do this enough that it matters?

Details

Assignee

Reporter

Labels

Approval

Incomplete

Patch

Code and Test

Priority

Affects versions

Created February 6, 2015 at 5:23 PM
Updated November 19, 2024 at 3:00 AM