You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucenenet.apache.org by ni...@apache.org on 2021/03/14 12:48:01 UTC

[lucenenet] branch master updated (5c62092 -> 0a3c227)

This is an automated email from the ASF dual-hosted git repository.

nightowl888 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git.


    from 5c62092  SWEEP: Reviewed catch clauses and made improvements to preserve stack details
     new b69b025  Lucene.Net.Facet.Taxonomy.CachedOrdinalsReader: Fixed synchronization issues with adding new items to the cache and reading RamBytesUsed() method (see #417)
     new c5c1dc0  Lucene.Net.Spatial.Util.ShapeFieldCacheProvider: Fixed atomicity issue with loading the cache by using Lazy<T>. Fixes #319. Also see #417.
     new e50e181  Lucene.Net.Util.VirtualMethod: Removed unnecessary call to Convert.ToInt32()
     new 0a3c227  Lucene.Net.Util.AttributeSource: Restored comment from Lucene indicating it doesn't matter if multiple threads compete to populate the ConditionalWeakTable. See #417.

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../Taxonomy/CachedOrdinalsReader.cs               | 56 +++++++---------------
 .../Util/ShapeFieldCacheProvider.cs                | 16 ++++---
 src/Lucene.Net/Util/AttributeSource.cs             |  1 +
 src/Lucene.Net/Util/VirtualMethod.cs               |  2 +-
 4 files changed, 28 insertions(+), 47 deletions(-)


[lucenenet] 04/04: Lucene.Net.Util.AttributeSource: Restored comment from Lucene indicating it doesn't matter if multiple threads compete to populate the ConditionalWeakTable. See #417.

Posted by ni...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

nightowl888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git

commit 0a3c227c5003725bcf8f7fee0d48f22bb8c8a7e6
Author: Shad Storhaug <sh...@shadstorhaug.com>
AuthorDate: Fri Mar 12 18:17:33 2021 +0700

    Lucene.Net.Util.AttributeSource: Restored comment from Lucene indicating it doesn't matter if multiple threads compete to populate the ConditionalWeakTable. See #417.
---
 src/Lucene.Net/Util/AttributeSource.cs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Lucene.Net/Util/AttributeSource.cs b/src/Lucene.Net/Util/AttributeSource.cs
index 91fb3f8..43ff27d 100644
--- a/src/Lucene.Net/Util/AttributeSource.cs
+++ b/src/Lucene.Net/Util/AttributeSource.cs
@@ -315,6 +315,7 @@ namespace Lucene.Net.Util
         {
             return knownImplClasses.GetValue(clazz, (key) =>
             {
+                // we have the slight chance that another thread may do the same, but who cares?
                 LinkedList<WeakReference<Type>> foundInterfaces = new LinkedList<WeakReference<Type>>();
                 // find all interfaces that this attribute instance implements
                 // and that extend the Attribute interface


[lucenenet] 02/04: Lucene.Net.Spatial.Util.ShapeFieldCacheProvider: Fixed atomicity issue with loading the cache by using Lazy. Fixes #319. Also see #417.

Posted by ni...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

nightowl888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git

commit c5c1dc09661da4f2fd3c6caf59044c4b1db8071c
Author: Shad Storhaug <sh...@shadstorhaug.com>
AuthorDate: Fri Mar 12 17:30:48 2021 +0700

    Lucene.Net.Spatial.Util.ShapeFieldCacheProvider: Fixed atomicity issue with loading the cache by using Lazy<T>. Fixes #319. Also see #417.
---
 src/Lucene.Net.Spatial/Util/ShapeFieldCacheProvider.cs | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/Lucene.Net.Spatial/Util/ShapeFieldCacheProvider.cs b/src/Lucene.Net.Spatial/Util/ShapeFieldCacheProvider.cs
index 4bbb278..3c9f011 100644
--- a/src/Lucene.Net.Spatial/Util/ShapeFieldCacheProvider.cs
+++ b/src/Lucene.Net.Spatial/Util/ShapeFieldCacheProvider.cs
@@ -1,7 +1,8 @@
-using Lucene.Net.Index;
+using Lucene.Net.Index;
 using Lucene.Net.Search;
 using Lucene.Net.Util;
 using Spatial4n.Core.Shapes;
+using System;
 using System.Runtime.CompilerServices;
 
 namespace Lucene.Net.Spatial.Util
@@ -39,8 +40,10 @@ namespace Lucene.Net.Spatial.Util
     {
         //private Logger log = Logger.GetLogger(GetType().FullName);
 
-        private readonly ConditionalWeakTable<IndexReader, ShapeFieldCache<T>> sidx =
-            new ConditionalWeakTable<IndexReader, ShapeFieldCache<T>>();
+        // LUCENENET specific - use Lazy<T> to ensure only 1 thread can call the createValueCallback at a time,
+        // since the default behavior is not atomic. See https://github.com/apache/lucenenet/issues/417.
+        private readonly ConditionalWeakTable<IndexReader, Lazy<ShapeFieldCache<T>>> sidx =
+            new ConditionalWeakTable<IndexReader, Lazy<ShapeFieldCache<T>>>();
 
         protected internal readonly int m_defaultSize;
         protected internal readonly string m_shapeField;
@@ -56,8 +59,9 @@ namespace Lucene.Net.Spatial.Util
 
         public virtual ShapeFieldCache<T> GetCache(AtomicReader reader)
         {
-            // LUCENENET: ConditionalWeakTable allows us to simplify and remove locks
-            return sidx.GetValue(reader, (key) =>
+            // LUCENENET: ConditionalWeakTable allows us to simplify and remove locks on the
+            // read operation. For the create case, we use Lazy<T> to ensure atomicity.
+            return sidx.GetValue(reader, (key) => new Lazy<ShapeFieldCache<T>>(() =>
             {
                 /*long startTime = Runtime.CurrentTimeMillis();
                 log.Fine("Building Cache [" + reader.MaxDoc() + "]");*/
@@ -88,7 +92,7 @@ namespace Lucene.Net.Spatial.Util
                 /*long elapsed = Runtime.CurrentTimeMillis() - startTime;
                 log.Fine("Cached: [" + count + " in " + elapsed + "ms] " + idx);*/
                 return idx;
-            });
+            })).Value;
         }
     }
 }


[lucenenet] 01/04: Lucene.Net.Facet.Taxonomy.CachedOrdinalsReader: Fixed synchronization issues with adding new items to the cache and reading RamBytesUsed() method (see #417)

Posted by ni...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

nightowl888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git

commit b69b02574b8aaebeac125d9f75b1fe8247f724d5
Author: Shad Storhaug <sh...@shadstorhaug.com>
AuthorDate: Fri Mar 12 17:20:14 2021 +0700

    Lucene.Net.Facet.Taxonomy.CachedOrdinalsReader: Fixed synchronization issues with adding new items to the cache and reading RamBytesUsed() method (see #417)
---
 .../Taxonomy/CachedOrdinalsReader.cs               | 56 +++++++---------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs b/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs
index 7ff6132..fa87607 100644
--- a/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs
+++ b/src/Lucene.Net.Facet/Taxonomy/CachedOrdinalsReader.cs
@@ -66,11 +66,10 @@ namespace Lucene.Net.Facet.Taxonomy
 
 #if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR
         private readonly ConditionalWeakTable<object, CachedOrds> ordsCache = new ConditionalWeakTable<object, CachedOrds>();
-        private readonly object ordsCacheLock = new object();
 #else
         private readonly WeakDictionary<object, CachedOrds> ordsCache = new WeakDictionary<object, CachedOrds>();
-        private readonly ReaderWriterLockSlim syncLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);
 #endif
+        private readonly object syncLock = new object();
 
         /// <summary>
         /// Sole constructor. </summary>
@@ -81,35 +80,23 @@ namespace Lucene.Net.Facet.Taxonomy
 
         private CachedOrds GetCachedOrds(AtomicReaderContext context)
         {
-            object cacheKey = context.Reader.CoreCacheKey;
-#if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR
-            return ordsCache.GetValue(cacheKey, (cacheKey) => new CachedOrds(source.GetReader(context), context.Reader.MaxDoc));
-#else
-            CachedOrds ords;
-            syncLock.EnterReadLock();
-            try
+            // LUCENENET NOTE: Since ConditionalWeakTable doesn't synchronize on enumeration in the RamBytesUsed() method,
+            // the lock is still required here despite it being a threadsafe collection.
+            lock (syncLock)
             {
-                if (ordsCache.TryGetValue(cacheKey, out ords))
-                    return ords;
-            }
-            finally
-            {
-                syncLock.ExitReadLock();
-            }
+                object cacheKey = context.Reader.CoreCacheKey;
+                if (!ordsCache.TryGetValue(cacheKey, out CachedOrds ords))
+                {
+                    ords = new CachedOrds(source.GetReader(context), context.Reader.MaxDoc);
 
-            ords = new CachedOrds(source.GetReader(context), context.Reader.MaxDoc);
-            syncLock.EnterWriteLock();
-            try
-            {
-                ordsCache[cacheKey] = ords;
-            }
-            finally
-            {
-                syncLock.ExitWriteLock();
+                    // LUCENENET specific: Since this is the only thread that can modify ordsCache
+                    // and we just checked that the value doesn't exist above, we can simplify this to Add()
+                    // which also means we don't need conditional compilation because ConditionalWeakTable
+                    // doesn't support this[index].
+                    ordsCache.Add(cacheKey, ords);
+                }
+                return ords;
             }
-
-            return ords;
-#endif
         }
 
         public override string IndexFieldName => source.IndexFieldName;
@@ -211,12 +198,7 @@ namespace Lucene.Net.Facet.Taxonomy
 
         public virtual long RamBytesUsed()
         {
-#if FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR
-            lock (ordsCacheLock)
-#else
-            syncLock.EnterReadLock();
-            try
-#endif
+            lock (syncLock)
             {
                 long bytes = 0;
                 foreach (var pair in ordsCache)
@@ -226,12 +208,6 @@ namespace Lucene.Net.Facet.Taxonomy
 
                 return bytes;
             }
-#if !FEATURE_CONDITIONALWEAKTABLE_ENUMERATOR
-            finally
-            {
-                syncLock.ExitReadLock();
-            }
-#endif
         }
     }
 }
\ No newline at end of file


[lucenenet] 03/04: Lucene.Net.Util.VirtualMethod: Removed unnecessary call to Convert.ToInt32()

Posted by ni...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

nightowl888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git

commit e50e1812596cd3146e33d67efa2d293216f9f941
Author: Shad Storhaug <sh...@shadstorhaug.com>
AuthorDate: Fri Mar 12 17:38:54 2021 +0700

    Lucene.Net.Util.VirtualMethod: Removed unnecessary call to Convert.ToInt32()
---
 src/Lucene.Net/Util/VirtualMethod.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Lucene.Net/Util/VirtualMethod.cs b/src/Lucene.Net/Util/VirtualMethod.cs
index 464ef50..c294037 100644
--- a/src/Lucene.Net/Util/VirtualMethod.cs
+++ b/src/Lucene.Net/Util/VirtualMethod.cs
@@ -147,7 +147,7 @@ namespace Lucene.Net.Util
         public int GetImplementationDistance(Type subclazz)
         {
             // LUCENENET: Replaced WeakIdentityMap with ConditionalWeakTable - This operation is simplified over Lucene.
-            return cache.GetValue(subclazz, (key) => Convert.ToInt32(ReflectImplementationDistance(key), CultureInfo.InvariantCulture));
+            return cache.GetValue(subclazz, (key) => ReflectImplementationDistance(key));
         }
 
         /// <summary>