Persistent collections updates #174

lorentey · 2022-09-08T04:14:34Z

Start ingressing the persistent collections code.

Review, update benchmarks
Reformat code to follow Standard Library conventions
Rename internals to follow The Leading Underscore Rule
Start reorganizing type definitions per stdlib conventions
Prefer nested types to top-level generics (wip)

There is no functional change as of yet.

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.

Follow the Standard Library coding style: - Hard 80 character line limit - Use two spaces as the indentation unit - Overlong lines broken up by adding line breaks at the following places 1. After opening delimiters & before closing delimiters 2. Between each item in a list 3. Extracting subexpressions etc as needed

This doesn’t need to be public API.

Also, move types into their own files.

- DictionaryKeyValueTupleIterator → PersistentDictionary.Iterator - DictionaryKeyValueTupleReverseIterator → PersistentDictionary.ReverseIterator The latter type is unused. Moved under the Unused folder for now.

…nary._ Node, Header → _NodeHeader Also move Capacity, Bitmap under _NodeHeader. Start thematically organizing _Node members.

lorentey · 2022-09-08T04:17:55Z

(The code formatting changes and large scale renamings/reorganizations make this effectively non-reviewable, I know. 😔)

…ry.Index

msteindorfer

LGTM

See clarifications and minor comments.

Tests/PersistentCollectionsTests/PersistentCollections Utils.swift

Tests/PersistentCollectionsTests/_CollidableInt.swift

msteindorfer · 2022-09-08T10:02:10Z

Sources/PersistentCollections/_NonzeroBits.swift

+//
+//===----------------------------------------------------------------------===//
+
+extension FixedWidthInteger {


The minimal requirement would be BinaryInteger for the trailingZeroBitCount property.

Suggested change

extension FixedWidthInteger {

extension BinaryInteger {

Sure, however, we don't need to over-generalize things -- I kept the ability to change Bitmap to a different type (for now), but I don't expect we'll ever exercise this freedom, and we definitely don't change it to a non-fixed-width integer type. (_NonzerBits is not public.)

msteindorfer · 2022-09-08T10:02:51Z

Sources/PersistentCollections/_NonzeroBits.swift

+  }
+}
+
+internal struct _NonzeroBits<Bitmap: FixedWidthInteger>


Suggested change

internal struct _NonzeroBits<Bitmap: FixedWidthInteger>

internal struct _NonzeroBits<Bitmap: BinaryInteger>

msteindorfer · 2022-09-08T10:12:30Z

Sources/PersistentCollections/Unused/_BaseIterator.swift

+#if false
+/// Base class for fixed-stack iterators that traverse a hash-trie. The iterator
+/// performs a depth-first pre-order traversal, which yields first all payload
+/// elements of the current node before traversing sub-nodes (left to right).
+internal struct _BaseIterator<T: _Node> {


The _BaseIterator and _BaseReverseIterator code generically against the higher level _Node protocol instead against the persistent dictionary internals.

The main rationale of keeping the generic _BaseIterator was to abstract over the PersistentDictionary and (future) PersistentSet that may vary in terms of internal representation.

For now this struct is indeed unused, since the PersistentDictionary contains a lower level iterator implementation, coding against it's internal implementation.

msteindorfer · 2022-09-08T10:25:54Z

Sources/PersistentCollections/_Common.swift

@@ -75,36 +60,6 @@ func rangeInsert<T>(
  src.initialize(to: element)
 }

-// `index` is the logical index starting at the rear, indexing to the left
-func rangeInsertReversed<T>(


The range(Insert|Remove)Reversed functions may need to be restored once we switch back to the native CHAMP ordering where the buffer grows from both ends inwards (similar to heap and stack).

(At the moment the dictionary node caches the offset to the sub-node section and grows all buffer segments from left to right, which is a relic of experimenting with various explicit hash-collision resolution strategies. )

msteindorfer · 2022-09-08T11:03:14Z

Sources/PersistentCollections/PersistentDictionary._Node.swift

-  }
+    var header: _NodeHeader
+    var count: Int

-  deinit {
-    dataBaseAddress.deinitialize(count: header.dataCount)
-    trieBaseAddress.deinitialize(count: header.trieCount)
+    let dataCapacity: Capacity
+    let trieCapacity: Capacity


The count and the capacity fields should eventually also move in the header, to have division between header and buffer akin to ManagedBuffer.

Then it also makes send to have the Capacity typealias inside the _NodeHeader.

Co-authored-by: Michael Steindorfer <michael@steindorfer.name>

lorentey added 10 commits September 7, 2022 17:35

[PersistentCollections] Add Xcode scheme

9b0aa87

[PersistentCollections] Review & update benchmarks

1a87233

[PersistentCollections] Move BitmapSmokeTests into its own file

7e78002

[PersistentCollections] NonzeroBits → _NonzeroBits

b2d6d98

This doesn’t need to be public API.

[PersistentCollections] Apply the leading underscore rule

8e19b80

Also, move types into their own files.

[PersistentDictionary] Clean up Iterator types

978a833

- DictionaryKeyValueTupleIterator → PersistentDictionary.Iterator - DictionaryKeyValueTupleReverseIterator → PersistentDictionary.ReverseIterator The latter type is unused. Moved under the Unused folder for now.

[PersistentDictionary] Remove unused utility functions

1f7eb2f

[PersistentDictionary] Apply leading underscore rule to common functions

2597ff2

[PersistentDictionary] BitmapIndexedDictionaryNode → PersistentDictio…

6f93a62

…nary._ Node, Header → _NodeHeader Also move Capacity, Bitmap under _NodeHeader. Start thematically organizing _Node members.

lorentey requested a review from msteindorfer September 8, 2022 04:14

[PersistentDictionary] PersistentDictionaryIndex → PersistentDictiona…

750559e

…ry.Index

msteindorfer approved these changes Sep 8, 2022

View reviewed changes

Update spelling of publication dates

aa428d0

Co-authored-by: Michael Steindorfer <michael@steindorfer.name>

lorentey merged commit ef29f79 into apple:feature/PersistentCollections Sep 8, 2022

lorentey deleted the PersistentCollections-updates branch September 8, 2022 23:04

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 #174

Persistent collections updates #174

lorentey commented Sep 8, 2022

lorentey commented Sep 8, 2022

msteindorfer left a comment

msteindorfer Sep 8, 2022

lorentey Sep 8, 2022

msteindorfer Sep 8, 2022

msteindorfer Sep 8, 2022

msteindorfer Sep 8, 2022

msteindorfer Sep 8, 2022

	internal struct _NonzeroBits<Bitmap: FixedWidthInteger>
	internal struct _NonzeroBits<Bitmap: BinaryInteger>

Persistent collections updates #174

Persistent collections updates #174

Conversation

lorentey commented Sep 8, 2022

Checklist

lorentey commented Sep 8, 2022

msteindorfer left a comment

Choose a reason for hiding this comment

msteindorfer Sep 8, 2022

Choose a reason for hiding this comment

lorentey Sep 8, 2022

Choose a reason for hiding this comment

msteindorfer Sep 8, 2022

Choose a reason for hiding this comment

msteindorfer Sep 8, 2022

Choose a reason for hiding this comment

msteindorfer Sep 8, 2022

Choose a reason for hiding this comment

msteindorfer Sep 8, 2022

Choose a reason for hiding this comment