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: dev-unsubscr...@lucenenet.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to