You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2021/02/04 10:41:41 UTC
[GitHub] [lucenenet] NightOwl888 commented on a change in pull request #410: remove facetconfig locks
NightOwl888 commented on a change in pull request #410:
URL: https://github.com/apache/lucenenet/pull/410#discussion_r570114823
##########
File path: src/Lucene.Net.Facet/FacetsConfig.cs
##########
@@ -157,17 +153,14 @@ public virtual DimConfig GetDimConfig(string dimName)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public virtual void SetHierarchical(string dimName, bool v)
{
- lock (syncLock)
+ // LUCENENET: Eliminated extra lookup by using TryGetValue instead of ContainsKey
+ if (!fieldTypes.TryGetValue(dimName, out DimConfig fieldType))
Review comment:
After removing the lock, these operations are no longer atomic.
However you are correct that the `ConcurrentDictionary` can be used to replace the lock by calling the appropriate overload of [`AddOrUpdate`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.addorupdate?view=net-5.0). Please change to use this `AddOrUpdate` rather than multiple method calls on the `ConcurrentDictionary`.
##########
File path: src/Lucene.Net.Facet/FacetsConfig.cs
##########
@@ -178,17 +171,14 @@ public virtual void SetHierarchical(string dimName, bool v)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public virtual void SetMultiValued(string dimName, bool v)
{
- lock (syncLock)
+ // LUCENENET: Eliminated extra lookup by using TryGetValue instead of ContainsKey
+ if (!fieldTypes.TryGetValue(dimName, out DimConfig fieldType))
Review comment:
After removing the lock, these operations are no longer atomic.
However you are correct that the `ConcurrentDictionary` can be used to replace the lock by calling the appropriate overload of [`AddOrUpdate`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.addorupdate?view=net-5.0). Please change to use this `AddOrUpdate` rather than multiple method calls on the `ConcurrentDictionary`.
##########
File path: src/Lucene.Net.Facet/FacetsConfig.cs
##########
@@ -221,17 +208,14 @@ public virtual void SetRequireDimCount(string dimName, bool v)
/// </summary>
public virtual void SetIndexFieldName(string dimName, string indexFieldName)
{
- lock (syncLock)
+ // LUCENENET: Eliminated extra lookup by using TryGetValue instead of ContainsKey
+ if (!fieldTypes.TryGetValue(dimName, out DimConfig fieldType))
Review comment:
After removing the lock, these operations are no longer atomic.
However you are correct that the `ConcurrentDictionary` can be used to replace the lock by calling the appropriate overload of [`AddOrUpdate`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.addorupdate?view=net-5.0). Please change to use this `AddOrUpdate` rather than multiple method calls on the `ConcurrentDictionary`.
##########
File path: src/Lucene.Net.Facet/FacetsConfig.cs
##########
@@ -200,17 +190,14 @@ public virtual void SetMultiValued(string dimName, bool v)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public virtual void SetRequireDimCount(string dimName, bool v)
{
- lock (syncLock)
+ // LUCENENET: Eliminated extra lookup by using TryGetValue instead of ContainsKey
+ if (!fieldTypes.TryGetValue(dimName, out DimConfig fieldType))
Review comment:
After removing the lock, these operations are no longer atomic.
However you are correct that the `ConcurrentDictionary` can be used to replace the lock by calling the appropriate overload of [`AddOrUpdate`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.addorupdate?view=net-5.0). Please change to use this `AddOrUpdate` rather than multiple method calls on the `ConcurrentDictionary`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org