Improved performance of @UniqueElements(by=NOT_SET)`#491
Improved performance of @UniqueElements(by=NOT_SET)`#491jlink merged 1 commit intojqwik-team:mainfrom
Conversation
| // instead of `items instanceof HashSet`, because subclasses | ||
| // of HashSet may break Set conventions. | ||
| return items -> items.getClass().equals(HashSet.class) | ||
| || items.size() == items.stream().distinct().count(); |
There was a problem hiding this comment.
This is a suboptimal way to check uniqueness. You can stop as soon as you detect the first non-unique element.
At the same time, elements in hashset are always unique, aren't they?
There was a problem hiding this comment.
Oh You're right, it would be much faster to stop early. I will fix that.
Yes that's why added the HashSet clause. It might speed things up sometimes.
There was a problem hiding this comment.
Changed the logic. Now it should stop early.
There was a problem hiding this comment.
Nice. There's one more case doing stream.distinct.count:
I wonder if it makes sense to unify them.
WDYT?
|
@DirkToewe As soon as you deem it ready, just rebase it, so that I can merge it. |
|
I did a soft reset and re-committed the changes. All merge conflicts should be gone now. The PR now also includes the change to |
|
Published in 1.7.4-SNAPSHOT |
Overview
This PR aims at significantly improving the performance of
@UniqueElements(by=NOT_SET), mainly by replacing calls touniqueElements(x->x)byuniqueElements()wherever possible.Details
If we look at the following two property tests:
On my machine, it takes 20 seconds for
testAto finish whiletestBonly takes 2 seconds. The problem becomes much more emphasized with larger array sizes.I hereby agree to the terms of the jqwik Contributor Agreement.