From 720806b66183dc55589beb9161bdea071ac9c6fa Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Sat, 15 Jun 2013 00:34:45 +0100 Subject: Adjust the locking on InventoryCache. Locking for r/w of the ExpiringCache isn't needed since it's thread safe but r/w of contained dictionaries isn't thread-safe --- .../Inventory/InventoryCache.cs | 37 ++++++++++++++-------- 1 file changed, 23 insertions(+), 14 deletions(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs index 1e434b9..dcf33a1 100644 --- a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs +++ b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs @@ -1,11 +1,14 @@ using System; using System.Collections.Generic; - +using System.Threading; using OpenSim.Framework; using OpenMetaverse; namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Inventory { + /// + /// Cache root and system inventory folders to reduce number of potentially remote inventory calls and associated holdups. + /// public class InventoryCache { private const double CACHE_EXPIRATION_SECONDS = 3600.0; // 1 hour @@ -16,8 +19,7 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Inventory public void Cache(UUID userID, InventoryFolderBase root) { - lock (m_RootFolders) - m_RootFolders.AddOrUpdate(userID, root, CACHE_EXPIRATION_SECONDS); + m_RootFolders.AddOrUpdate(userID, root, CACHE_EXPIRATION_SECONDS); } public InventoryFolderBase GetRootFolder(UUID userID) @@ -31,14 +33,18 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Inventory public void Cache(UUID userID, AssetType type, InventoryFolderBase folder) { - lock (m_FolderTypes) + Dictionary ff = null; + if (!m_FolderTypes.TryGetValue(userID, out ff)) + { + ff = new Dictionary(); + m_FolderTypes.Add(userID, ff, CACHE_EXPIRATION_SECONDS); + } + + // We need to lock here since two threads could potentially retrieve the same dictionary + // and try to add a folder for that type simultaneously. Dictionary<>.Add() is not described as thread-safe in the SDK + // even if the folders are identical. + lock (ff) { - Dictionary ff = null; - if (!m_FolderTypes.TryGetValue(userID, out ff)) - { - ff = new Dictionary(); - m_FolderTypes.Add(userID, ff, CACHE_EXPIRATION_SECONDS); - } if (!ff.ContainsKey(type)) ff.Add(type, folder); } @@ -50,8 +56,12 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Inventory if (m_FolderTypes.TryGetValue(userID, out ff)) { InventoryFolderBase f = null; - if (ff.TryGetValue(type, out f)) - return f; + + lock (ff) + { + if (ff.TryGetValue(type, out f)) + return f; + } } return null; @@ -59,8 +69,7 @@ namespace OpenSim.Region.CoreModules.ServiceConnectorsOut.Inventory public void Cache(UUID userID, InventoryCollection inv) { - lock (m_Inventories) - m_Inventories.AddOrUpdate(userID, inv, 120); + m_Inventories.AddOrUpdate(userID, inv, 120); } public InventoryCollection GetUserInventory(UUID userID) -- cgit v1.1 From ecfc6a3f4a44b70c3d7b3b03126f29f246136dfa Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Sat, 15 Jun 2013 00:36:04 +0100 Subject: Add the standard OpenSimulator copyright notice to the top of InventoryCache.cs --- .../Inventory/InventoryCache.cs | 29 +++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs index dcf33a1..2fc8ee3 100644 --- a/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs +++ b/OpenSim/Region/CoreModules/ServiceConnectorsOut/Inventory/InventoryCache.cs @@ -1,4 +1,31 @@ -using System; +/* + * Copyright (c) Contributors, http://opensimulator.org/ + * See CONTRIBUTORS.TXT for a full list of copyright holders. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the OpenSimulator Project nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE CONTRIBUTORS BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +using System; using System.Collections.Generic; using System.Threading; using OpenSim.Framework; -- cgit v1.1 From 9c530d725f13da91caffdbce2759d58a456afff5 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Sat, 15 Jun 2013 00:41:02 +0100 Subject: refactor: In UserProfileModule, change classifiedCache and classifiedInterest to m_classifiedCache and m_classifiedInterest This is the coding standard name style for private fields. --- .../Avatar/UserProfiles/UserProfileModule.cs | 42 ++++++++++++---------- 1 file changed, 23 insertions(+), 19 deletions(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs index 322addd..97bb781 100644 --- a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs @@ -60,8 +60,8 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles // The pair of Dictionaries are used to handle the switching of classified ads // by maintaining a cache of classified id to creator id mappings and an interest // count. The entries are removed when the interest count reaches 0. - Dictionary classifiedCache = new Dictionary(); - Dictionary classifiedInterest = new Dictionary(); + Dictionary m_classifiedCache = new Dictionary(); + Dictionary m_classifiedInterest = new Dictionary(); public Scene Scene { @@ -331,16 +331,19 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles classifieds[cid] = name; - if(!classifiedCache.ContainsKey(cid)) + lock(m_classifiedCache) { - lock(classifiedCache) - classifiedCache.Add(cid,creatorId); - lock(classifiedInterest) - classifiedInterest.Add(cid, 0); + if (!m_classifiedCache.ContainsKey(cid)) + { + m_classifiedCache.Add(cid,creatorId); + + lock(m_classifiedInterest) + m_classifiedInterest.Add(cid, 0); + } } - lock(classifiedInterest) - classifiedInterest[cid] ++; + lock(m_classifiedInterest) + m_classifiedInterest[cid]++; } remoteClient.SendAvatarClassifiedReply(new UUID(args[0]), classifieds); @@ -352,19 +355,20 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles UserClassifiedAdd ad = new UserClassifiedAdd(); ad.ClassifiedId = queryClassifiedID; - if(classifiedCache.ContainsKey(queryClassifiedID)) + lock (classifie + if (m_classifiedCache.ContainsKey(queryClassifiedID)) { - target = classifiedCache[queryClassifiedID]; + target = m_classifiedCache[queryClassifiedID]; - lock(classifiedInterest) - classifiedInterest[queryClassifiedID] --; + lock(m_classifiedInterest) + m_classifiedInterest[queryClassifiedID] --; - if(classifiedInterest[queryClassifiedID] == 0) + if(m_classifiedInterest[queryClassifiedID] == 0) { - lock(classifiedInterest) - classifiedInterest.Remove(queryClassifiedID); - lock(classifiedCache) - classifiedCache.Remove(queryClassifiedID); + lock(m_classifiedInterest) + m_classifiedInterest.Remove(queryClassifiedID); + lock(m_classifiedCache) + m_classifiedCache.Remove(queryClassifiedID); } } @@ -1339,4 +1343,4 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles } #endregion Web Util } -} +} \ No newline at end of file -- cgit v1.1 From 42b0c68eabcd79ea1d7d5776b399bcbb46bbbf98 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Sat, 15 Jun 2013 00:46:55 +0100 Subject: Correct build break in previous commit 9c530d7 --- .../Avatar/UserProfiles/UserProfileModule.cs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs index 97bb781..d7ffea8 100644 --- a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs @@ -60,8 +60,8 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles // The pair of Dictionaries are used to handle the switching of classified ads // by maintaining a cache of classified id to creator id mappings and an interest // count. The entries are removed when the interest count reaches 0. - Dictionary m_classifiedCache = new Dictionary(); - Dictionary m_classifiedInterest = new Dictionary(); + Dictionary m_classifiedCache = new Dictionary(); + Dictionary m_classifiedInterest = new Dictionary(); public Scene Scene { @@ -102,7 +102,7 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles /// /// Gets or sets a value indicating whether this - /// is enabled. + /// is enabled. /// /// /// true if enabled; otherwise, false. @@ -331,15 +331,13 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles classifieds[cid] = name; - lock(m_classifiedCache) + if (!m_classifiedCache.ContainsKey(cid)) { - if (!m_classifiedCache.ContainsKey(cid)) - { + lock(m_classifiedCache) m_classifiedCache.Add(cid,creatorId); - lock(m_classifiedInterest) - m_classifiedInterest.Add(cid, 0); - } + lock(m_classifiedInterest) + m_classifiedInterest.Add(cid, 0); } lock(m_classifiedInterest) @@ -355,7 +353,6 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles UserClassifiedAdd ad = new UserClassifiedAdd(); ad.ClassifiedId = queryClassifiedID; - lock (classifie if (m_classifiedCache.ContainsKey(queryClassifiedID)) { target = m_classifiedCache[queryClassifiedID]; -- cgit v1.1 From e6cb7b47646f8d7a23aca50d92fb57a639bbf702 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Sat, 15 Jun 2013 00:52:57 +0100 Subject: Lock m_classifiedCache and m_classifiedInterest dictionary reads in UserProfileModule since in the presence of writes these are not thread-safe operations. Simplified locking to m_classifiedCache only since r/w of both dictionaries always occurs together --- .../Avatar/UserProfiles/UserProfileModule.cs | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs index d7ffea8..161f160 100644 --- a/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/UserProfiles/UserProfileModule.cs @@ -331,17 +331,16 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles classifieds[cid] = name; - if (!m_classifiedCache.ContainsKey(cid)) + lock (m_classifiedCache) { - lock(m_classifiedCache) + if (!m_classifiedCache.ContainsKey(cid)) + { m_classifiedCache.Add(cid,creatorId); - - lock(m_classifiedInterest) m_classifiedInterest.Add(cid, 0); - } + } - lock(m_classifiedInterest) m_classifiedInterest[cid]++; + } } remoteClient.SendAvatarClassifiedReply(new UUID(args[0]), classifieds); @@ -353,19 +352,19 @@ namespace OpenSim.Region.OptionalModules.Avatar.UserProfiles UserClassifiedAdd ad = new UserClassifiedAdd(); ad.ClassifiedId = queryClassifiedID; - if (m_classifiedCache.ContainsKey(queryClassifiedID)) - { - target = m_classifiedCache[queryClassifiedID]; + lock (m_classifiedCache) + { + if (m_classifiedCache.ContainsKey(queryClassifiedID)) + { + target = m_classifiedCache[queryClassifiedID]; - lock(m_classifiedInterest) m_classifiedInterest[queryClassifiedID] --; - if(m_classifiedInterest[queryClassifiedID] == 0) - { - lock(m_classifiedInterest) + if (m_classifiedInterest[queryClassifiedID] == 0) + { m_classifiedInterest.Remove(queryClassifiedID); - lock(m_classifiedCache) m_classifiedCache.Remove(queryClassifiedID); + } } } -- cgit v1.1