Enhance the performance of map merges

Description

It would be nice if merge used transients.

Patch

  • clj-1458-7.patch

Approach
Migrate c.c/merge later in core after transients & reduce. Leave older version as merge1 for use in cases the precede the newer definition. Make APersistentMap/conj & ATransientMap/cons aware of IKVReduce.

The attached patch preserves two existing behaviors of merge

  • metadata propagation

  • the right hand side of the merges can be a Map.Entry, an IPersistentVector where size=2, and regular maps.

Screened by:

Environment

None

Activity

Show:
Alex Miller
February 1, 2016, 11:17 AM

Can you update the ticket approach section to discuss the APersistentMap.cons / ASSOC changes. Also, can you add a before / after perf test for one or more common cases?

Michael Blume
September 28, 2016, 7:55 PM

Updated patch to handle use of merge in core_print before it's defined in core

Ghadi Shayban
September 28, 2016, 8:22 PM

If anyone wants to take stewardship of this, go ahead. I had trouble getting consistent performance improvements on this. Obviously this needs fresh benchmarks.

Alex Miller
September 28, 2016, 8:28 PM

Yes, this needs a benchmark showing demonstrable improvement. The whole goal here is improved perf - if we can't prove it's consistently faster, then there is no point in even reviewing it.

Ghadi Shayban
November 28, 2017, 10:40 PM

This ticket needs help. Step 0 is writing a benchmark harness that exercises maps of various sizes, and the various polymorphic arguments below.

After a benchmark harness, implementation code needs to preserve this behavior:

  • If the first argument is not nil, its metadata propagates to the result.

  • The arguments can be a Map.Entry, an IPersistentVector where size=2, and regular maps.

  • When passed a non-map as the first argument, result is undefined.

Assignee

Unassigned

Reporter

import

Labels

Approval

Triaged

Patch

Code and Test

Priority

Major