Skip to content

Conversation

@lorentey
Copy link
Member

@lorentey lorentey commented Sep 9, 2021

In debug builds, the OrderedSet.init(uncheckedUniqueElements:) initializer always created a set with no hash table, even if it had an element count >15. This is technically an invariant violation (caught by _checkInvariants()), although it generally only triggers (very) slow performance. (The set reverts to linear searching.)

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.

@lorentey lorentey added this to the 1.0.0 milestone Sep 9, 2021
@lorentey
Copy link
Member Author

lorentey commented Sep 9, 2021

(This was uncovered while trying to reproduce #104; it's a real bug, but sadly it's a performance issue, not a correctness bug like that one.)

@lorentey
Copy link
Member Author

lorentey commented Sep 9, 2021

@swift-ci test

…ash table

In debug builds, the `OrderedSet.init(uncheckedUniqueElements:)` initializer always created a set with no hash table, even if it had an element count >15. This is technically an invariant violation (caught by `_checkInvariants()`), although it generally only triggers (very) slow performance. (The set reverts to linear searching.)
…second argument

This will hopefully prevent issues like the one fixed in the previous commit from reoccurring in the future.
@lorentey
Copy link
Member Author

lorentey commented Sep 9, 2021

Rebased on top of latest changes on release/1.0 to satisfy branch protection rules.

@swift-ci test

@lorentey lorentey merged commit 37e15ae into apple:release/1.0 Sep 10, 2021
@lorentey lorentey deleted the let’s-table-that branch September 10, 2021 18:15
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