NightOwl888 commented on code in PR #938:
URL: https://github.com/apache/lucenenet/pull/938#discussion_r1597773408
##########
src/Lucene.Net/Support/ConcurrentHashSet.cs:
##########
@@ -753,7 +753,16 @@ private void CopyToItems(T[] array, int index)
public void ExceptWith(IEnumerable<T> other)
{
- throw new NotImplementedException();
+ if (ReferenceEquals(this, other))
Review Comment:
My concern is less about whether this will succeed with FSync, and more
about whether the `ConcurrentHashSet<T>` matches the behavior in Java, as it
may be eventually cleaned up, moved to J2N, and have a full suite of Harmony +
dotnet/runtime tests added to confirm it works as expected.
Synchronizing around the entire set is what is done in Java. They use a
wrapper class that locks access to the entire set. But IMO, we are better off
emulating the `ConcurrentDictionary` in .NET, which uses locked "buckets". The
whole set can be locked by locking all of the buckets.
So, like what was done on `UnionWith`, if we are calling into existing code,
we should separate the internal operations from the locking code so we can call
into the business logic under different locking conditions. In this case, we
should be locking all of the buckets at the beginning of the method call and
then calling `InternalRemove` (which does no locking).
> I don't think there's any amount of concurrent-izing this hashset that
avoids these potential race conditions. The only proper solution would be to
synchronize around the entire hashset, at which point we might as well just use
the BCL non-concurrent HashSet instead. Let me know what you think.
I guess that is also an option. The `ConcurrentSet` is just such a wrapper
that locks the entire set as was done in Java. It can be used by calling `new
HashSet<T>().AsConcurrent()`.
But, it mostly only exists because of other JDK concerns such as structural
equality, structural formatting, and because these `ISet<T>` operations are
fully implemented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
