BUG: Lucene.Net.Core.Util.WeakIdentityMap: Added fix for WeakIdentityMap and TestWeakIdentityMap provided by Vincent Van Den Berghe
Project: http://git-wip-us.apache.org/repos/asf/lucenenet/repo Commit: http://git-wip-us.apache.org/repos/asf/lucenenet/commit/74e166f6 Tree: http://git-wip-us.apache.org/repos/asf/lucenenet/tree/74e166f6 Diff: http://git-wip-us.apache.org/repos/asf/lucenenet/diff/74e166f6 Branch: refs/heads/api-work Commit: 74e166f626464253439291600238bea8b55c07d0 Parents: e8bf549 Author: Shad Storhaug <[email protected]> Authored: Tue Mar 21 23:55:24 2017 +0700 Committer: Shad Storhaug <[email protected]> Committed: Tue Mar 21 23:55:24 2017 +0700 ---------------------------------------------------------------------- src/Lucene.Net.Core/Util/WeakIdentityMap.cs | 119 ++++++++----------- .../Util/TestWeakIdentityMap.cs | 40 ++++--- 2 files changed, 72 insertions(+), 87 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucenenet/blob/74e166f6/src/Lucene.Net.Core/Util/WeakIdentityMap.cs ---------------------------------------------------------------------- diff --git a/src/Lucene.Net.Core/Util/WeakIdentityMap.cs b/src/Lucene.Net.Core/Util/WeakIdentityMap.cs index c6c35c4..2999d94 100644 --- a/src/Lucene.Net.Core/Util/WeakIdentityMap.cs +++ b/src/Lucene.Net.Core/Util/WeakIdentityMap.cs @@ -1,29 +1,27 @@ -using Lucene.Net.Support; using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Linq; using System.Runtime.CompilerServices; using System.Collections; namespace Lucene.Net.Util { /* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ /// <summary> /// Implements a combination of <seealso cref="java.util.WeakHashMap"/> and @@ -61,9 +59,8 @@ namespace Lucene.Net.Util /// @lucene.internal /// </summary> public sealed class WeakIdentityMap<TKey, TValue> - where TKey : class + where TKey : class { - //private readonly ReferenceQueue<object> queue = new ReferenceQueue<object>(); private readonly IDictionary<IdentityWeakReference, TValue> backingStore; private readonly bool reapOnRead; @@ -82,7 +79,7 @@ namespace Lucene.Net.Util /// <param name="reapOnRead"> controls if the map <a href="#reapInfo">cleans up the reference queue on every read operation</a>. </param> public static WeakIdentityMap<TKey, TValue> NewHashMap(bool reapOnRead) { - return new WeakIdentityMap<TKey, TValue>(new HashMap<IdentityWeakReference, TValue>(), reapOnRead); + return new WeakIdentityMap<TKey, TValue>(new Dictionary<IdentityWeakReference, TValue>(), reapOnRead); } /// <summary> @@ -252,16 +249,17 @@ namespace Lucene.Net.Util private class IteratorAnonymousInnerClassHelper : IEnumerator<TKey> { - private readonly WeakIdentityMap<TKey,TValue> outerInstance; + private readonly WeakIdentityMap<TKey, TValue> outerInstance; + private readonly IEnumerator<KeyValuePair<IdentityWeakReference, TValue>> enumerator; - public IteratorAnonymousInnerClassHelper(WeakIdentityMap<TKey,TValue> outerInstance) + public IteratorAnonymousInnerClassHelper(WeakIdentityMap<TKey, TValue> outerInstance) { this.outerInstance = outerInstance; + enumerator = outerInstance.backingStore.GetEnumerator(); } // holds strong reference to next element in backing iterator: private object next = null; - private int position = -1; // start before the beginning of the set public TKey Current { @@ -279,56 +277,32 @@ namespace Lucene.Net.Util } } + public void Dispose() { - // Nothing to do + enumerator.Dispose(); } - + public bool MoveNext() { - while (true) + while (enumerator.MoveNext()) { - IdentityWeakReference key; - - // If the next position doesn't exist, exit - if (++position >= outerInstance.backingStore.Count) - { - position--; - return false; - } - try + next = enumerator.Current.Key.Target; + if (next != null) { - key = outerInstance.backingStore.Keys.ElementAt(position); - } - catch (ArgumentOutOfRangeException) - { - // some other thread beat us to the last element (or removed a prior element) - fail gracefully. - position--; - return false; - } - if (!key.IsAlive) - { - outerInstance.backingStore.Remove(key); - position--; - continue; - } - // unfold "null" special value: - if (key.Target == NULL) - { - next = null; - } - else - { - next = key.Target; + // unfold "null" special value: + if (next == NULL) + next = null; + return true; } - return true; } + return false; } public void Reset() { - throw new NotSupportedException(); + enumerator.Reset(); } } @@ -354,22 +328,27 @@ namespace Lucene.Net.Util /// collected key/value pairs from the map. Calling this method is not needed /// if {@code reapOnRead = true}. Otherwise it might be a good idea /// to call this method when there is spare time (e.g. from a background thread). </summary> - /// <seealso cref= <a href="#reapInfo">Information about the <code>reapOnRead</code> setting</a> </seealso> + /// <seealso cref= <a href="#reapInfo">Information about the <code>reapOnRead</code> setting</a> </seealso> public void Reap() { - List<IdentityWeakReference> keysToRemove = new List<IdentityWeakReference>(); - foreach (IdentityWeakReference zombie in backingStore.Keys) + List<IdentityWeakReference> keysToRemove = null; + foreach (var item in backingStore) { - if (!zombie.IsAlive) + if (!item.Key.IsAlive) { - keysToRemove.Add(zombie); + // create the list of keys to remove only if there are keys to remove. + // this reduces heap pressure + if (keysToRemove == null) + keysToRemove = new List<IdentityWeakReference>(); + keysToRemove.Add(item.Key); } } - foreach (var key in keysToRemove) - { - backingStore.Remove(key); - } + if (keysToRemove != null) + foreach (var key in keysToRemove) + { + backingStore.Remove(key); + } } // we keep a hard reference to our NULL key, so map supports null keys that never get GCed: @@ -396,9 +375,9 @@ namespace Lucene.Net.Util { return true; } - if (o is IdentityWeakReference) + IdentityWeakReference @ref = o as IdentityWeakReference; + if (@ref != null) { - IdentityWeakReference @ref = (IdentityWeakReference)o; if (this.Target == @ref.Target) { return true; http://git-wip-us.apache.org/repos/asf/lucenenet/blob/74e166f6/src/Lucene.Net.Tests/Util/TestWeakIdentityMap.cs ---------------------------------------------------------------------- diff --git a/src/Lucene.Net.Tests/Util/TestWeakIdentityMap.cs b/src/Lucene.Net.Tests/Util/TestWeakIdentityMap.cs index ac8cd70..48270e0 100644 --- a/src/Lucene.Net.Tests/Util/TestWeakIdentityMap.cs +++ b/src/Lucene.Net.Tests/Util/TestWeakIdentityMap.cs @@ -1,3 +1,4 @@ +using Lucene.Net.Attributes; using Lucene.Net.Randomized.Generators; using Lucene.Net.Support; using NUnit.Framework; @@ -56,11 +57,13 @@ namespace Lucene.Net.Util // try null key & check its iterator also return null: map.Put(null, "null"); { - IEnumerator<string> iter = map.Keys.GetEnumerator(); - Assert.IsTrue(iter.MoveNext()); - Assert.IsNull(iter.Current); - Assert.IsFalse(iter.MoveNext()); - Assert.IsFalse(iter.MoveNext()); + using (IEnumerator<string> iter = map.Keys.GetEnumerator()) + { + Assert.IsTrue(iter.MoveNext()); + Assert.IsNull(iter.Current); + Assert.IsFalse(iter.MoveNext()); + Assert.IsFalse(iter.MoveNext()); + } } // 2 more keys: map.Put(key1, "bar1"); @@ -105,10 +108,9 @@ namespace Lucene.Net.Util Assert.AreEqual(3, map.Count); int c = 0, keysAssigned = 0; - for (IEnumerator<string> iter = map.Keys.GetEnumerator(); iter.MoveNext();) + foreach (object k in map.Keys) { //Assert.IsTrue(iter.hasNext()); // try again, should return same result! - string k = iter.Current; // LUCENENET NOTE: Need object.ReferenceEquals here because the == operator does more than check reference equality Assert.IsTrue(object.ReferenceEquals(k, key1) || object.ReferenceEquals(k, key2) | object.ReferenceEquals(k, key3)); keysAssigned += object.ReferenceEquals(k, key1) ? 1 : (object.ReferenceEquals(k, key2) ? 2 : 4); @@ -143,9 +145,9 @@ namespace Lucene.Net.Util size = newSize; Thread.Sleep(TimeSpan.FromSeconds(1)); c = 0; - for (IEnumerator<string> iter = map.Keys.GetEnumerator(); iter.MoveNext();) + foreach (object k in map.Keys) { - Assert.IsNotNull(iter.Current); + Assert.IsNotNull(k); c++; } newSize = map.Count; @@ -166,8 +168,8 @@ namespace Lucene.Net.Util Assert.AreEqual(0, map.Count); Assert.IsTrue(map.IsEmpty); - IEnumerator<string> it = map.Keys.GetEnumerator(); - Assert.IsFalse(it.MoveNext()); + using (IEnumerator<string> it = map.Keys.GetEnumerator()) + Assert.IsFalse(it.MoveNext()); /*try { it.Next(); @@ -194,7 +196,11 @@ namespace Lucene.Net.Util Assert.IsTrue(map.IsEmpty); } - [Test] +#if !NETSTANDARD + // LUCENENET: There is no Timeout on NUnit for .NET Core. + [Timeout(60000)] +#endif + [Test, HasTimeout] public virtual void TestConcurrentHashMap() { // don't make threadCount and keyCount random, otherwise easily OOMs or fails otherwise: @@ -224,7 +230,7 @@ namespace Lucene.Net.Util { foreach (var w in workers) { - w.Join(1000L); + w.Join(); } } @@ -258,9 +264,9 @@ namespace Lucene.Net.Util size = newSize; Thread.Sleep(new TimeSpan(100L)); int c = 0; - for (IEnumerator<object> it = map.Keys.GetEnumerator(); it.MoveNext();) + foreach (object k in map.Keys) { - Assert.IsNotNull(it.Current); + Assert.IsNotNull(k); c++; } newSize = map.Count; @@ -332,9 +338,9 @@ namespace Lucene.Net.Util break; case 4: // check iterator still working - for (IEnumerator<object> it = map.Keys.GetEnumerator(); it.MoveNext();) + foreach (object k in map.Keys) { - Assert.IsNotNull(it.Current); + Assert.IsNotNull(k); } break; default:
