Persistent collections updates (part 5) #179

lorentey · 2022-09-18T01:14:06Z

Reverse the ordering of items in node storage, fixing copying colliding items in iteration order
Indices now represent paths into the hash tree instead of the preorder position of an item. This speeds up looking up indices (and the item addressed by an index), but it slows down index(after:) and index(before:).
Add index validation logic.
Add a way to dump trees in preorder walk order
Implement in-place mutations for the key subscript and the defaulted subscript. (This was a pain in the neck to implement.)
Add many more tests.
Fix a logic oversight in the node sizing logic that lead to nodes unnecessarily doubling their size with each new generation of copies.
Use the term "slot" rather than "offset" for positions within internal storage.

Checklist

I've read the Contribution Guidelines
My contributions are licensed under the Swift license.
I've followed the coding style of the rest of the project.
I've added tests covering all new code paths my change adds to the project (if appropriate).
I've added benchmarks covering new functionality (if appropriate).
I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
I've updated the documentation if necessary.

If the key implements hashing properly, then this would be a clever way to try another hash seed to differentiate between colliding keys. However, if Key has proper hashing, then collision nodes are expected to be tiny, so it’s better to simply do naive comparisons. If the Key type has broken (i.e., partial) hashing, then loading colliding items into a dictionary would just make == even slower. So we’re better off comparing collidingh items via nested loops — so let’s just unconditionally do that.

These provide a barebones, read-only, non-generic abstraction, with just enough functionality to allow navigating around in the tree structure. We will use these to implement path-based indices.

The Index type now represents a path into the tree, consisting of a series of child offsets from the root node, as well as the item offset within the final node. (This all fits in a couple of words, as the depth of the tree is fixed, and the offset path can always be compressed into a single `UInt` value.) To speed up basic operations, we also cache the final node as an unsafe-unowned reference. Dereferencing this is extremely unsafe, so the Index type carries an unsafe reference to the root node along with a mutation counter, to allow reliable(*) index validation. If index’s stored root is identical to the root of the collection, and they have the same version value, then we can assume that the index is still valid.

The new fixtures provide an easier way to construct sample hash trees with specific structure, to run tests on them. Fixtures are able to determine the expected ordering for each sample tree, which comes super useful for verifying collection conformances.

(Internally) standardize on using the word “slot” to refer to an id for a location within a contiguous storage buffer that can hold a single item of some kind. “Offset” is sometimes used in public APIs (to refer to integer distances within collections, relative to some start position), and using the same term here causes internal naming difficulties. A “bucket” is therefore a more specific name for a slot within a hash table.

`RawCollider` doesn’t like to get called through the high-level hashing APIs.

…script

# Conflicts: # Tests/PersistentCollections2Tests/PersistentMap Fixtures.swift

Base the allocation request on the currently occupied space, not the full capacity. The original code tended to lead to unnecessary doubling of the node size in each copy, until we exhaust the 32 bit capacity value. (Oops.)

lorentey · 2022-09-18T01:15:00Z

@swift-ci test

lorentey · 2022-09-18T01:21:21Z

Iteration performance is boosted when the dictionary starts to go beyond a single layer of nodes.
(I suspect there is some constant overhead here that might be possible to eliminate. Retain/release traffic?)

lorentey · 2022-09-18T01:24:35Z

index(forKey:) performance is slower than I expect, reinforcing my suspicion about there being some bogus overhead. Looking at generated code will probably make it obvious where the time goes.

lorentey · 2022-09-18T01:30:05Z

The new in-place mutation support does show up as a bit of improvement on these microbenchmarks, but support for this is mostly exciting because it prevents CoW copies for the mutated value -- it's not necessary about raw lookup performance.

That said, in-place mutation for the defaulted subscript gets a solid 53% performance boost:

lorentey · 2022-09-18T01:31:47Z

Benchmark summary (compared to #177):

Tasks with difference scores larger than 1.05:
  Score   Sum     Improvements Regressions  Name
  17.51   17.51   17.51(#75)   0.9993(#1)   PersistentDictionary<Int, Int> indexing subscript (*)
  1.887   1.887   1.967(#55)   0.9207(#21)  PersistentDictionary<Int, Int>.Keys sequential iteration (*)
  1.881   1.881   1.963(#55)   0.9184(#21)  PersistentDictionary<Int, Int>.Values sequential iteration (*)
  1.738   1.738   1.870(#55)   0.8676(#21)  PersistentDictionary<Int, Int> sequential iteration, indices (*)
  1.596   0.626   1.001(#2)    0.6260(#74)  PersistentDictionary<Int, Int> unsuccessful index(forKey:) (*)
  1.532   1.532   1.532(#75)   0.9997(#1)   PersistentDictionary<Int, Int> defaulted subscript, _modify existing (*)
  1.108   1.108   1.113(#74)   0.9953(#2)   PersistentDictionary<Int, Int> subscript, _modify (*)
  1.086   1.086   1.099(#59)   0.9874(#17)  PersistentDictionary<Int, Int> random removals (missing keys) (*)
  1.081   1.081   1.082(#75)   0.9989(#1)   PersistentDictionary<Int, Int> defaulted subscript, _modify missing (*)
  1.065   1.065   1.069(#60)   0.9964(#16)  PersistentDictionary<Int, Int> subscript, remove existing, unique (*)
  1.064   0.940   1.010(#6)    0.9301(#70)  PersistentDictionary<Int, Int> updateValue(_:forKey:), insert (*)
  1.063   1.063   1.065(#70)   0.9981(#6)   PersistentDictionary<Int, Int> subscript, successful lookups (*)
  1.059   1.059   1.065(#65)   0.9943(#11)  PersistentDictionary<Int, Int> defaulted subscript, successful lookups (*)

lorentey · 2022-09-18T03:06:20Z

Last commit fixes index(forKey:) performance:

Tasks with difference scores larger than 1.05:
  Score   Sum     Improvements Regressions  Name
  17.33   17.33   17.33(#75)   0.9993(#1)   PersistentDictionary<Int, Int> indexing subscript (*)
  1.983   1.983   1.983(#76)   1.000(#0)    PersistentDictionary<Int, Int> successful index(forKey:) (*)
  1.867   1.867   1.949(#55)   0.9175(#21)  PersistentDictionary<Int, Int>.Keys sequential iteration (*)
  1.851   1.851   1.937(#55)   0.9142(#21)  PersistentDictionary<Int, Int>.Values sequential iteration (*)
  1.713   1.713   1.843(#55)   0.8699(#21)  PersistentDictionary<Int, Int> sequential iteration, indices (*)
  1.526   1.526   1.526(#76)   1.000(#0)    PersistentDictionary<Int, Int> defaulted subscript, _modify existing (*)
  1.272   1.272   1.272(#76)   1.000(#0)    PersistentDictionary<Int, Int> unsuccessful index(forKey:) (*)
  1.223   1.223   1.223(#76)   1.000(#0)    PersistentDictionary<Int, Int> random removals (missing keys) (*)
  1.184   1.184   1.184(#75)   1.000(#1)    PersistentDictionary<Int, Int> subscript, _modify (*)
  1.183   1.183   1.183(#76)   1.000(#0)    PersistentDictionary<Int, Int> subscript, noop setter (*)
  1.167   1.167   1.167(#76)   1.000(#0)    PersistentDictionary<Int, Int> subscript, remove missing (*)
  1.103   1.103   1.105(#64)   0.9983(#12)  PersistentDictionary<Int, Int> subscript, remove existing, unique (*)
  1.071   1.071   1.076(#66)   0.9957(#10)  PersistentDictionary<Int, Int> defaulted subscript, successful lookups (*)
  1.069   1.069   1.071(#71)   0.9978(#5)   PersistentDictionary<Int, Int> subscript, successful lookups (*)
  1.068   0.936   1.005(#10)   0.9312(#66)  PersistentDictionary<Int, Int> updateValue(_:forKey:), insert (*)
  1.065   1.065   1.067(#65)   0.9987(#11)  PersistentDictionary<Int, Int> random removals (existing keys) (*)

lorentey · 2022-09-18T03:06:41Z

@swift-ci test

msteindorfer

LGTM

Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift

Sources/PersistentCollections/Node/_Node.swift

Sources/PersistentCollections/Node/_Level.swift

Sources/PersistentCollections/Node/_Node+Mutations.swift

lorentey added 17 commits September 15, 2022 20:56

[PersistentCollections] Reverse ordering of items in node storage

2e523b1

[PersistentCollections] _Level: Store the shift amount in an UInt8

edaadc9

[PersistentCollections] Flesh out _RawNode, add _UnmanagedNode

6dd0211

These provide a barebones, read-only, non-generic abstraction, with just enough functionality to allow navigating around in the tree structure. We will use these to implement path-based indices.

[PersistentCollections] Rework basic node properties

547ac90

[PersistentCollections] Allow dumping hash trees in iteration order

16393ea

[test] LifetimeTracked: Implement high-fidelity hash forwarding

52e3ea6

`RawCollider` doesn’t like to get called through the high-level hashing APIs.

[PersistentCollections] Internal doc updates

6cf128e

[OrderedDictionary] Implement index invalidation

1324c4e

[PersistentDictionary] Implement in-place mutations for defaulted sub…

1661bd4

…script

[PersistentDictionary] Add some docs

3b7d241

[PersistentDictionary] Implement in-place mutations

57deed5

# Conflicts: # Tests/PersistentCollections2Tests/PersistentMap Fixtures.swift

[PersistentCollections] Fix node sizing logic

2321b34

Base the allocation request on the currently occupied space, not the full capacity. The original code tended to lead to unnecessary doubling of the node size in each copy, until we exhaust the 32 bit capacity value. (Oops.)

[PersistentCollections] Reduce _Bucket’s storage size

5240bf6

lorentey requested a review from msteindorfer September 18, 2022 01:14

[PersistentDictionary] Fix index(forKey:) performance

d45f264

msteindorfer approved these changes Sep 19, 2022

View reviewed changes

lorentey merged commit 7499bc2 into apple:feature/PersistentCollections Sep 19, 2022

lorentey deleted the PersistentCollections-updates branch September 19, 2022 17:56

lorentey added this to the 1.1.0 milestone Oct 10, 2022

lorentey added the HashTreeCollections Hash-array mapped prefix tree implementations label Nov 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persistent collections updates (part 5) #179

Persistent collections updates (part 5) #179

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

msteindorfer left a comment

Persistent collections updates (part 5) #179

Persistent collections updates (part 5) #179

Conversation

lorentey commented Sep 18, 2022

Checklist

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

lorentey commented Sep 18, 2022

msteindorfer left a comment

Choose a reason for hiding this comment