From 78141630b76c55deae2fcc554d2e1ed7d5c84243 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Sep 2014 11:37:26 +1000 Subject: [PATCH] After a lot of benchmarking i found the bottleneck for building the prop collection. Even though this new bottleneck is leaps and bounds faster than it was before... with this new fix it is epically fast --- .../Persistence/Factories/IEntityFactory.cs | 1 + .../Persistence/Factories/PropertyFactory.cs | 19 ++-- .../Repositories/ContentRepository.cs | 4 +- .../Repositories/MediaRepository.cs | 4 +- .../Repositories/MemberRepository.cs | 17 ++-- .../Repositories/VersionableRepositoryBase.cs | 95 ++++++++++--------- .../Services/ContentServicePerformanceTest.cs | 3 + .../ExamineManagementApiController.cs | 2 + 8 files changed, 78 insertions(+), 67 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Factories/IEntityFactory.cs b/src/Umbraco.Core/Persistence/Factories/IEntityFactory.cs index a0d8823d68..d413e30308 100644 --- a/src/Umbraco.Core/Persistence/Factories/IEntityFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/IEntityFactory.cs @@ -1,6 +1,7 @@ namespace Umbraco.Core.Persistence.Factories { //TODO: Not sure why we need this interface as it's never referenced in code. + //TODO: Delete this internal interface IEntityFactory where TEntity : class diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs index a3eee1369f..4e3653bf9e 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs @@ -6,37 +6,35 @@ using Umbraco.Core.Models.Rdbms; namespace Umbraco.Core.Persistence.Factories { - internal class PropertyFactory : IEntityFactory, IEnumerable> + internal class PropertyFactory { - private readonly IContentTypeComposition _contentType; + private readonly PropertyType[] _compositionTypeProperties; private readonly Guid _version; private readonly int _id; private readonly DateTime _createDate; private readonly DateTime _updateDate; - public PropertyFactory(IContentTypeComposition contentType, Guid version, int id) + public PropertyFactory(PropertyType[] compositionTypeProperties, Guid version, int id) { - _contentType = contentType; + _compositionTypeProperties = compositionTypeProperties; _version = version; _id = id; } - public PropertyFactory(IContentTypeComposition contentType, Guid version, int id, DateTime createDate, DateTime updateDate) + public PropertyFactory(PropertyType[] compositionTypeProperties, Guid version, int id, DateTime createDate, DateTime updateDate) { - _contentType = contentType; + _compositionTypeProperties = compositionTypeProperties; _version = version; _id = id; _createDate = createDate; _updateDate = updateDate; } - #region Implementation of IEntityFactory - - public IEnumerable BuildEntity(IEnumerable dtos) + public IEnumerable BuildEntity(PropertyDataDto[] dtos) { var properties = new List(); - foreach (var propertyType in _contentType.CompositionPropertyTypes) + foreach (var propertyType in _compositionTypeProperties) { var propertyDataDto = dtos.LastOrDefault(x => x.PropertyTypeId == propertyType.Id); var property = propertyDataDto == null @@ -104,6 +102,5 @@ namespace Umbraco.Core.Persistence.Factories return propertyDataDtos; } - #endregion } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 0577347661..a2ffd29bcf 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -313,7 +313,7 @@ namespace Umbraco.Core.Persistence.Repositories Database.Insert(dto); //Create the PropertyData for this version - cmsPropertyData - var propertyFactory = new PropertyFactory(entity.ContentType, entity.Version, entity.Id); + var propertyFactory = new PropertyFactory(entity.ContentType.CompositionPropertyTypes.ToArray(), entity.Version, entity.Id); var propertyDataDtos = propertyFactory.BuildDto(entity.Properties); var keyDictionary = new Dictionary(); @@ -446,7 +446,7 @@ namespace Umbraco.Core.Persistence.Repositories } //Create the PropertyData for this version - cmsPropertyData - var propertyFactory = new PropertyFactory(((Content)entity).ContentType, entity.Version, entity.Id); + var propertyFactory = new PropertyFactory(entity.ContentType.CompositionPropertyTypes.ToArray(), entity.Version, entity.Id); var propertyDataDtos = propertyFactory.BuildDto(entity.Properties); var keyDictionary = new Dictionary(); diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs index 4269132b97..c5966a5bc9 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs @@ -247,7 +247,7 @@ namespace Umbraco.Core.Persistence.Repositories Database.Insert(dto); //Create the PropertyData for this version - cmsPropertyData - var propertyFactory = new PropertyFactory(entity.ContentType, entity.Version, entity.Id); + var propertyFactory = new PropertyFactory(entity.ContentType.CompositionPropertyTypes.ToArray(), entity.Version, entity.Id); var propertyDataDtos = propertyFactory.BuildDto(entity.Properties); var keyDictionary = new Dictionary(); @@ -319,7 +319,7 @@ namespace Umbraco.Core.Persistence.Repositories Database.Update(dto); //Create the PropertyData for this version - cmsPropertyData - var propertyFactory = new PropertyFactory(entity.ContentType, entity.Version, entity.Id); + var propertyFactory = new PropertyFactory(entity.ContentType.CompositionPropertyTypes.ToArray(), entity.Version, entity.Id); var propertyDataDtos = propertyFactory.BuildDto(entity.Properties); var keyDictionary = new Dictionary(); diff --git a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs index 5c04cbcb46..0edd6eff2b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs @@ -243,7 +243,7 @@ namespace Umbraco.Core.Persistence.Repositories Database.Insert(dto); //Create the PropertyData for this version - cmsPropertyData - var propertyFactory = new PropertyFactory(entity.ContentType, entity.Version, entity.Id); + var propertyFactory = new PropertyFactory(entity.ContentType.CompositionPropertyTypes.ToArray(), entity.Version, entity.Id); //Add Properties // - don't try to save the property if it doesn't exist (or doesn't have an ID) on the content type // - this can occur if the member type doesn't contain the built-in properties that the @@ -345,7 +345,7 @@ namespace Umbraco.Core.Persistence.Repositories //TODO ContentType for the Member entity //Create the PropertyData for this version - cmsPropertyData - var propertyFactory = new PropertyFactory(entity.ContentType, entity.Version, entity.Id); + var propertyFactory = new PropertyFactory(entity.ContentType.CompositionPropertyTypes.ToArray(), entity.Version, entity.Id); var keyDictionary = new Dictionary(); //Add Properties @@ -628,8 +628,7 @@ namespace Umbraco.Core.Persistence.Repositories var dtos = Database.Fetch(sql); //content types - var contentTypes = _memberTypeRepository.GetAll(dtos.Select(x => x.ContentVersionDto.ContentDto.ContentTypeId).ToArray()) - .ToArray(); + var contentTypes = _memberTypeRepository.GetAll(dtos.Select(x => x.ContentVersionDto.ContentDto.ContentTypeId).ToArray()).ToArray(); var dtosWithContentTypes = dtos //This select into and null check are required because we don't have a foreign damn key on the contentType column @@ -639,19 +638,19 @@ namespace Umbraco.Core.Persistence.Repositories .ToArray(); //Go get the property data for each document - var docDefs = dtosWithContentTypes.Select(d => new DocumentDefinition( + IEnumerable docDefs = dtosWithContentTypes.Select(d => new DocumentDefinition( d.dto.NodeId, d.dto.ContentVersionDto.VersionId, d.dto.ContentVersionDto.VersionDate, d.dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, d.contentType)); - var propertyData = GetPropertyCollection(sql, docDefs); + var propertyData = GetPropertyCollection(sql, docDefs); return dtosWithContentTypes.Select(d => CreateMemberFromDto( - d.dto, - contentTypes.First(ct => ct.Id == d.dto.ContentVersionDto.ContentDto.ContentTypeId), - propertyData[d.dto.NodeId])); + d.dto, + contentTypes.First(ct => ct.Id == d.dto.ContentVersionDto.ContentDto.ContentTypeId), + propertyData[d.dto.NodeId])); } /// diff --git a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs index 3094e7dd41..9f453109ab 100644 --- a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Text; +using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.Models.EntityBase; @@ -370,9 +372,9 @@ INNER JOIN (" + string.Format(parsedOriginalSql, "cmsContent.nodeId, cmsContentVersion.VersionId") + @") as docData ON cmsPropertyData.versionId = docData.VersionId AND cmsPropertyData.contentNodeId = docData.nodeId LEFT OUTER JOIN cmsDataTypePreValues -ON cmsPropertyType.dataTypeId = cmsDataTypePreValues.datatypeNodeId", docSql.Arguments); +ON cmsPropertyType.dataTypeId = cmsDataTypePreValues.datatypeNodeId", docSql.Arguments); - var allPropertyData = Database.Fetch(propSql); + var allPropertyData = Database.Fetch(propSql); //This is a lazy access call to get all prevalue data for the data types that make up all of these properties which we use // below if any property requires tag support @@ -387,63 +389,70 @@ INNER JOIN (" + string.Format(parsedOriginalSql, "cmsContent.contentType") + @") as docData ON cmsPropertyType.contentTypeId = docData.contentType", docSql.Arguments); - return Database.Fetch(preValsSql); + return Database.Fetch(preValsSql); }); var result = new Dictionary(); var propertiesWithTagSupport = new Dictionary(); - foreach (var def in documentDefs) + //iterate each definition grouped by it's content type - this will mean less property type iterations while building + // up the property collections + foreach (var compositionGroup in documentDefs.GroupBy(x => x.Composition)) { - var propertyDataDtos = allPropertyData.Where(x => x.NodeId == def.Id).Distinct(); + var compositionProperties = compositionGroup.Key.CompositionPropertyTypes.ToArray(); - var propertyFactory = new PropertyFactory(def.Composition, def.Version, def.Id, def.CreateDate, def.VersionDate); - var properties = propertyFactory.BuildEntity(propertyDataDtos).ToArray(); - - var newProperties = properties.Where(x => x.HasIdentity == false && x.PropertyType.HasIdentity); - foreach (var property in newProperties) + foreach (var def in compositionGroup) { - var propertyDataDto = new PropertyDataDto { NodeId = def.Id, PropertyTypeId = property.PropertyTypeId, VersionId = def.Version }; - int primaryKey = Convert.ToInt32(Database.Insert(propertyDataDto)); + var propertyDataDtos = allPropertyData.Where(x => x.NodeId == def.Id).Distinct(); - property.Version = def.Version; - property.Id = primaryKey; - } + var propertyFactory = new PropertyFactory(compositionProperties, def.Version, def.Id, def.CreateDate, def.VersionDate); + var properties = propertyFactory.BuildEntity(propertyDataDtos.ToArray()).ToArray(); + + var newProperties = properties.Where(x => x.HasIdentity == false && x.PropertyType.HasIdentity); - foreach (var property in properties) - { - //NOTE: The benchmarks run with and without the following code show very little change so this is not a perf bottleneck - var editor = PropertyEditorResolver.Current.GetByAlias(property.PropertyType.PropertyEditorAlias); - - var tagSupport = propertiesWithTagSupport.ContainsKey(property.PropertyType.PropertyEditorAlias) - ? propertiesWithTagSupport[property.PropertyType.PropertyEditorAlias] - : TagExtractor.GetAttribute(editor); - - if (tagSupport != null) + foreach (var property in newProperties) { - //add to local cache so we don't need to reflect next time for this property editor alias - propertiesWithTagSupport[property.PropertyType.PropertyEditorAlias] = tagSupport; + var propertyDataDto = new PropertyDataDto { NodeId = def.Id, PropertyTypeId = property.PropertyTypeId, VersionId = def.Version }; + int primaryKey = Convert.ToInt32(Database.Insert(propertyDataDto)); - //this property has tags, so we need to extract them and for that we need the prevals which we've already looked up - var preValData = allPreValues.Value.Where(x => x.DataTypeNodeId == property.PropertyType.DataTypeDefinitionId) - .Distinct() - .ToArray(); - - var asDictionary = preValData.ToDictionary(x => x.Alias, x => new PreValue(x.Id, x.Value, x.SortOrder)); - - var preVals = new PreValueCollection(asDictionary); - - var contentPropData = new ContentPropertyData(property.Value, - preVals, - new Dictionary()); - - TagExtractor.SetPropertyTags(property, contentPropData, property.Value, tagSupport); + property.Version = def.Version; + property.Id = primaryKey; } - } + foreach (var property in properties) + { + //NOTE: The benchmarks run with and without the following code show very little change so this is not a perf bottleneck + var editor = PropertyEditorResolver.Current.GetByAlias(property.PropertyType.PropertyEditorAlias); - result.Add(def.Id, new PropertyCollection(properties)); + var tagSupport = propertiesWithTagSupport.ContainsKey(property.PropertyType.PropertyEditorAlias) + ? propertiesWithTagSupport[property.PropertyType.PropertyEditorAlias] + : TagExtractor.GetAttribute(editor); + + if (tagSupport != null) + { + //add to local cache so we don't need to reflect next time for this property editor alias + propertiesWithTagSupport[property.PropertyType.PropertyEditorAlias] = tagSupport; + + //this property has tags, so we need to extract them and for that we need the prevals which we've already looked up + var preValData = allPreValues.Value.Where(x => x.DataTypeNodeId == property.PropertyType.DataTypeDefinitionId) + .Distinct() + .ToArray(); + + var asDictionary = preValData.ToDictionary(x => x.Alias, x => new PreValue(x.Id, x.Value, x.SortOrder)); + + var preVals = new PreValueCollection(asDictionary); + + var contentPropData = new ContentPropertyData(property.Value, + preVals, + new Dictionary()); + + TagExtractor.SetPropertyTags(property, contentPropData, property.Value, tagSupport); + } + } + + result.Add(def.Id, new PropertyCollection(properties)); + } } return result; diff --git a/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs b/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs index 3dc2f5ca5d..3f2e22bf66 100644 --- a/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs +++ b/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs @@ -45,6 +45,9 @@ namespace Umbraco.Tests.Services // 5290ms !!!!!! // // that is a 96% savings of processing and sql calls! + // + // ... NOPE, made even more nice changes, it is now... + // 4452ms !!!!!!! var contentType1 = MockedContentTypes.CreateTextpageContentType("test1", "test1"); var contentType2 = MockedContentTypes.CreateTextpageContentType("test2", "test2"); diff --git a/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs b/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs index cf031fbd6a..3361bb43a6 100644 --- a/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs +++ b/src/Umbraco.Web/WebServices/ExamineManagementApiController.cs @@ -10,6 +10,7 @@ using Examine.LuceneEngine.Providers; using Examine.Providers; using Lucene.Net.Search; using Umbraco.Core; +using Umbraco.Core.Logging; using Umbraco.Web.Search; using Umbraco.Web.WebApi; @@ -121,6 +122,7 @@ namespace Umbraco.Web.WebServices } catch (Exception ex) { + LogHelper.Error("An error occurred rebuilding index", ex); var response = Request.CreateResponse(HttpStatusCode.Conflict); response.Content = new StringContent(string.Format("The index could not be rebuilt at this time, most likely there is another thread currently writing to the index. Error: {0}", ex)); response.ReasonPhrase = "Could Not Rebuild";