Skip to content

Optimize Set is_disjoint#531

Merged
bors merged 2 commits intorust-lang:masterfrom
ToMe25:optimize_is_disjoint
Jun 11, 2024
Merged

Optimize Set is_disjoint#531
bors merged 2 commits intorust-lang:masterfrom
ToMe25:optimize_is_disjoint

Conversation

@ToMe25
Copy link
Copy Markdown
Contributor

@ToMe25 ToMe25 commented Jun 7, 2024

By using the Intersection iterator in HashSet::is_Disjoint its performance can be significantly improved in some cases.
This is because intersection() always uses the shorter set as its iterator.

It would also be possible to replicate this "Iterate over the smaller set and check in the larger set" logic in the is_disjoint method.
However in my benchmarks the approach I chose is faster than that.

This change only causes a significant improvement when called on the larger of two disjoint sets.

My benchmark results:

Name Before After Diff (%)
disjoint_is_disjoint_large_small 1,147.43 535.25 -53,35 %
disjoint_is_disjoint_small_large 535.66 527.59 -1,51 %
subset_is_disjoint 9.90 10.44 5,45 %
superset_is_disjoint 9.80 10.43 6,43 %

@Amanieu
Copy link
Copy Markdown
Member

Amanieu commented Jun 10, 2024

@bors r+

@bors
Copy link
Copy Markdown
Contributor

bors commented Jun 10, 2024

📌 Commit fc448cf has been approved by Amanieu

It is now in the queue for this repository.

bors added a commit that referenced this pull request Jun 10, 2024
Optimize Set is_disjoint

By using the `Intersection` iterator in `HashSet::is_Disjoint` its performance can be significantly improved in some cases.
This is because `intersection()` always uses the shorter set as its iterator.

It would also be possible to replicate this "Iterate over the smaller set and check in the larger set" logic in the is_disjoint method.
However in my benchmarks the approach I chose is faster than that.

This change only causes a significant improvement when called on the larger of two disjoint sets.

My benchmark results:

Name | Before | After | Diff (%)
-- | -- | -- | --
disjoint_is_disjoint_large_small | 1,147.43 | 535.25 | -53,35 %
disjoint_is_disjoint_small_large | 535.66 | 527.59 | -1,51 %
subset_is_disjoint | 9.90 | 10.44 | 5,45 %
superset_is_disjoint | 9.80 | 10.43 | 6,43 %
@bors
Copy link
Copy Markdown
Contributor

bors commented Jun 10, 2024

⌛ Testing commit fc448cf with merge 62dd521...

@bors
Copy link
Copy Markdown
Contributor

bors commented Jun 10, 2024

💔 Test failed - checks-actions

@ToMe25
Copy link
Copy Markdown
Contributor Author

ToMe25 commented Jun 10, 2024

That error seems to be a Dead Code error which I'm pretty sure this PR couldn't have caused, is there anything I can/should do about this?

@Amanieu
Copy link
Copy Markdown
Member

Amanieu commented Jun 10, 2024

Just add #[cfg(feature = "raw")] to these types. The CI failure is probably due to lint changes in nightly rustc.

@ToMe25
Copy link
Copy Markdown
Contributor Author

ToMe25 commented Jun 10, 2024 via email

This will hopefully fix the CI error.
@ToMe25
Copy link
Copy Markdown
Contributor Author

ToMe25 commented Jun 11, 2024

Hooray that actually fixed CI.

@Amanieu
Copy link
Copy Markdown
Member

Amanieu commented Jun 11, 2024

@bors r+

@bors
Copy link
Copy Markdown
Contributor

bors commented Jun 11, 2024

📌 Commit 4b6e11d has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Copy Markdown
Contributor

bors commented Jun 11, 2024

⌛ Testing commit 4b6e11d with merge e25e6bb...

@bors
Copy link
Copy Markdown
Contributor

bors commented Jun 11, 2024

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing e25e6bb to master...

@bors bors merged commit e25e6bb into rust-lang:master Jun 11, 2024
@ToMe25 ToMe25 deleted the optimize_is_disjoint branch June 11, 2024 15:09
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.

3 participants