umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.4k stars 2.66k forks source link

Backoffice slows down getting content with a lot of variants #14936

Open dawoe opened 11 months ago

dawoe commented 11 months ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.6.1 to 12.x

Bug summary

We have a site with 6 languages. We also have uMarketing suite installed. For some pages multiple segment variants are created with uMarketing suite.

We noticed while adding more languages, and the customer creating personalized content with uMarketingSuite this slows down loading of content in the backoffice. The frontend runs just fine.

We noticed that getting a response from this API method is slow : https://github.com/umbraco/Umbraco-CMS/blob/cd2855634eb4528d003ec7080e3c8aa0002bc5e2/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs#L460

On our live environment this takes up between 8 to 10 seconds to return a result. With the copy of the database of the live environment on our local machine this takes between 5 and 7 seconds.

Adding more variants (culture or segment) increases the response time. Removing variants decreases the response time.

We see the same behavior when use this database on a clean Umbraco install. So we can rule out uMarketing suite.

Specifics

I did a debugging session against the Umbraco source code and found where the majority of time is spent.

It happens in this method : https://github.com/umbraco/Umbraco-CMS/blob/cd2855634eb4528d003ec7080e3c8aa0002bc5e2/src/Umbraco.Core/Models/Mapping/ContentPropertyDisplayMapper.cs#L35

This mapping function is called for each property in each variant. Because this happens in a loop we are executing code that is the same for all properties in all variants. For example getting the datatype configuration. But this is the same for all variants, so there is no need to get it over and over again.

This can be probably be improved by getting all this information earlier in the mapping process and pass it on using the MappingContext.

Steps to reproduce

Create a site with a lot of culture variants. If needed install uMarketingSuite and some personalized content as well.

See the performance of the GetById degrading with each variant added.

Expected result / actual result

Adding more variants should not slow down the retrieval of content significantly.

github-actions[bot] commented 11 months ago

Hi there @dawoe!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

Migaroez commented 10 months ago

Hey @dawoe, Thx for the clear and complete bug description and investigating the possible culprit, going to have a look at this and will get back to you asap.

ronaldbarendse commented 10 months ago

We've implemented performance improvements in mapping code for a similar issue in PR https://github.com/umbraco/Umbraco-CMS/pull/10521 by storing/caching data in the MapperContext, exactly like you already mentioned. Calling DataTypeService.GetDataType() for every property (even if some use the same data type), multiplied by every culture and again multiplied by every segment does indeed cause a lot of unnecessary overhead...

Migaroez commented 10 months ago

Intermediate findings: Tested on v10/dev(132935c) Used the following code https://gist.githubusercontent.com/Migaroez/162faab59d4d04515d665e27f50e2735/raw/e25436167c3882118699760c0760912bf244ff78/ContentGenerateController And payload

{
    "amount":3,
    "doctypeAliases":["fifteenCultureDocType"],
    "containingDoctypeAlias":"container",
    "containtingDoctypeUsesListview" : "false",
    "newDocTypePropertyMinAmount":30,
    "newDocTypePropertyMaxAmount":30,
    "languageIsoCodes":["en-us","nl-nl","nl-be","fr-fr","dk-dk","fr-be","fr-en","aa","aa-DJ","aa-ER","aa-ET","af","af-NA","af-ZA","agq"],
    "segments" :["segmentOne","segmentTwo","segmentThree","segmentFour","segmentFive"]
}

Takes around 426ms to get one of these nodes trough the Backoffice on SQL Server Dev 16 and 289ms on sqllite Without cultures, it takes around 57ms on sqllite.

So there definitely is an impact related to enabling and using segments but not anything dramatic as Dave is seeing. Granted, this is on a limited data set with only textbox properties.

Will continue the investigation

Migaroez commented 10 months ago

Increasing the nodes to 1000+ does not seem to mess with the median response time, but there are some random spikes, this might indicate some caching issues. image

Since reproducing this on a blank installation seems to be not straight forward, I am going to request some dedicated time to investigate this in a future sprint.

dawoe commented 10 months ago

Hi @Migaroez and @ronaldbarendse

Did some further digging myself. It's the mapping of BlockList properties that is taking up all the time on my end. We have several of them. And some of the blocks in them even have blocklist properties.

If I prevent the following lines being executed for the block list editor my page loads in about 500ms.

https://github.com/umbraco/Umbraco-CMS/blob/contrib/src/Umbraco.Core/Models/Mapping/ContentPropertyBasicMapper.cs#L94

https://github.com/umbraco/Umbraco-CMS/blob/contrib/src/Umbraco.Core/Models/Mapping/ContentPropertyDisplayMapper.cs#L50

https://github.com/umbraco/Umbraco-CMS/blob/contrib/src/Umbraco.Core/Models/Mapping/ContentPropertyDisplayMapper.cs#L78

So there is something fishy going on when getting the data for the block list editor.

cornehoskam commented 10 months ago

After trying out the possible pullrequest by @dawoe, the performance seems to be improved for all intents and purposes that use the ContentService to fetch content from a uMarketingSuite perspective, especially when fetching content items in bulk.

dawoe commented 10 months ago

I just did some digging. Our customer started reporting a slow backoffice after we upgraded from 10.2.x to 10.4.x

jmcaveney commented 9 months ago

I've looked a bit at this, since we're having similar issues. we have 6 languages and nodes with three different block editors on them, and the load times for GetById are sometimes 14+ seconds on Umbraco Cloud for those nodes.

As @ronaldbarendse mentioned, the loading of the datatypes, specifically this call on line 213, seem to be the cause: https://github.com/umbraco/Umbraco-CMS/blob/release/10.8/src/Umbraco.Infrastructure/PropertyEditors/BlockEditorPropertyValueEditor.cs#L213

This is done multiple times for every block property, even if the data type configurations are shared across blocks. Checking the dictionary of IDataValueEditors before calling to the database to get the data types seems to improve the load times significantly.

        int dataTypeId = prop.Value.PropertyType.DataTypeId;
        if (!valEditors.TryGetValue(dataTypeId, out IDataValueEditor? valEditor) || valEditor == null)
        {
            IDataType? dataType = _dataTypeService.GetDataType(dataTypeId);
            if (dataType == null)
            {
                // deal with weird situations by ignoring them (no comment)
                row.PropertyValues.Remove(prop.Key);
                _logger.LogWarning(
                    "ToEditor removed property value {PropertyKey} in row {RowId} for property type {PropertyTypeAlias}",
                    prop.Key,
                    row.Key,
                    property.PropertyType.Alias);
                continue;
            }

            var tempConfig = dataType.Configuration;
            valEditor = propEditor.GetValueEditor(tempConfig);

            valEditors.Add(dataTypeId, valEditor);
        }
        // update the raw value since this is what will get serialized out
        row.RawPropertyValues[prop.Key] = valEditor.ToEditor(tempProp);

It's not, however, the entirety of the problem. It seems to knock the load times by about 30% but they're still pretty slow. I plan to look at it further next week when I'm actually in the office and on the same network as the SQL server I'm referencing.

I would assume we could cache those data type calls across all block editors per content request. Otherwise we're loading data types for every language and every block editor and then for both settings and content sections of each block editor.

techcolin commented 8 months ago

I'd like to chime in on this issue but from a different perspective. Apologies as I don't want to cause confusion, but need to see if there's a relationship here.

Our client is on v8.18.7 and has an instance with 17 language variants. There are serious issues with backoffice performance we've not reported as we've been due to upgrade since Feb last year. With delays we're now confirmed to be upgrading to v13 shortly. I'm concerned the issue we're seeing will still exist, as this ticket seems to correspond to what we're seeing.

To summarise, when we load a page in the BO GetById can take generally 10 to 17 seconds to return. The speed depends on the number of blocks, but at best we're seing 7-8 seconds load time for any page.

It seems like Umbraco is loading ALL content for ALL variants despite the BO admin being in a single culture context for the editor.

Can anyone confirm if the upgrade is likely to solve anything here before we go too far? If we can assist with additional info please let me know as I appreciate this is quite an unusual site that makes heavier use of variants than most.

bielu commented 8 months ago

@dawoe I spend time playing around with backoffice and this happens not only when you have a lot variants(I have just one variant), but also when you have a lot nested blocks in grid block list editor... my times just by introducing simple decorator such as:

public class DataTypeServiceCacheDecorator : IDataTypeService
{
    private readonly IDataTypeService _dataTypeService;
    private readonly IAppPolicyCache _runtimeCache;

    public DataTypeServiceCacheDecorator(IDataTypeService dataTypeService, AppCaches caches)
    {
        _dataTypeService = dataTypeService;
        _runtimeCache = caches.RuntimeCache;
    }

    private string GetCacheKey(string operationName, params object[] args) =>
        $"DataTypeServiceCacheDecorator_{operationName}_{string.Join("_", args)}";
    private T GetCachedResult<T>(string cacheKey, Func<T> operation) =>
        _runtimeCache.GetCacheItem(cacheKey, operation, TimeSpan.FromMilliseconds(20));
    public IReadOnlyDictionary<Udi, IEnumerable<string>> GetReferences(int id) => GetCachedResult(GetCacheKey(nameof(this.GetReferences),id), ()=>_dataTypeService.GetReferences(id));

    public Attempt<OperationResult<OperationResultType, EntityContainer>?> CreateContainer(int parentId, Guid key, string name, int userId = -1) =>this._dataTypeService.CreateContainer(parentId, key, name, userId);

    public Attempt<OperationResult?> SaveContainer(EntityContainer container, int userId = -1) =>
        this._dataTypeService.SaveContainer(container, userId);

    public EntityContainer? GetContainer(int containerId) => GetCachedResult(GetCacheKey(nameof(this.GetContainer),containerId), ()=>_dataTypeService.GetContainer(containerId));

    public EntityContainer? GetContainer(Guid containerId) => GetCachedResult(GetCacheKey(nameof(this.GetContainer),containerId), ()=>_dataTypeService.GetContainer(containerId));

    public IEnumerable<EntityContainer> GetContainers(string folderName, int level) => GetCachedResult(GetCacheKey(nameof(this.GetContainers),folderName,level), ()=>_dataTypeService.GetContainers(folderName,level));

    public IEnumerable<EntityContainer> GetContainers(IDataType dataType) => GetCachedResult(GetCacheKey(nameof(this.GetContainers),dataType.Id,dataType.ParentId), ()=>_dataTypeService.GetContainers(dataType));

    public IEnumerable<EntityContainer> GetContainers(int[] containerIds) => GetCachedResult(GetCacheKey(nameof(this.GetContainers),containerIds), ()=>_dataTypeService.GetContainers(containerIds));

    public Attempt<OperationResult?> DeleteContainer(int containerId, int userId = -1) => this._dataTypeService.DeleteContainer(containerId, userId);

    public Attempt<OperationResult<OperationResultType, EntityContainer>?> RenameContainer(int id, string name, int userId = -1) => this._dataTypeService.RenameContainer(id, name, userId);

    public IDataType? GetDataType(string name) =>GetCachedResult(GetCacheKey(nameof(this.GetDataType),name), ()=>_dataTypeService.GetDataType(name));

    public IDataType? GetDataType(int id) =>GetCachedResult(GetCacheKey(nameof(this.GetDataType),id), ()=>_dataTypeService.GetDataType(id));

    public IDataType? GetDataType(Guid id) =>GetCachedResult(GetCacheKey(nameof(this.GetDataType),id), ()=>_dataTypeService.GetDataType(id));

    public IEnumerable<IDataType> GetAll(params int[] ids) =>GetCachedResult(GetCacheKey(nameof(this.GetAll),ids), ()=>_dataTypeService.GetAll(ids));

    public void Save(IDataType dataType, int userId = -1) => this._dataTypeService.Save(dataType, userId);

    public void Save(IEnumerable<IDataType> dataTypeDefinitions, int userId = -1) => this._dataTypeService.Save(dataTypeDefinitions, userId);

    public void Delete(IDataType dataType, int userId = -1) => this._dataTypeService.Delete(dataType, userId);

    public IEnumerable<IDataType> GetByEditorAlias(string propertyEditorAlias) =>GetCachedResult(GetCacheKey(nameof(this.GetByEditorAlias),propertyEditorAlias), ()=>_dataTypeService.GetByEditorAlias(propertyEditorAlias));

    public Attempt<OperationResult<MoveOperationStatusType>?> Move(IDataType toMove, int parentId) => this._dataTypeService.Move(toMove, parentId);
}

went down from from 30s to just 8... so yeah something fishy is going on with block editor

techcolin commented 8 months ago

@bielu which version is this on?

bielu commented 8 months ago

@techcolin I am on v13.0.3

techcolin commented 8 months ago

Thanks @bielu for confirming. This is bad news that I'll need to break to our client, and I suspect not an easy thing to re-architect from @UmbracoHQ's point of view.

It would be good to get Umbraco's feedback on this one. Given that we're on v8, and yet there seems to be further slowdown within v10 as reported above, and ongoing performance issues on v13 I think we're in trouble.

For what it's worth, this time last year we increased our VM to 4-core 16GB ram to try and improve things, and even went on to a Premium Azure SQL tier (very expensive) to see how much it helped. Things improved, but not to the degree we hoped. Without running any specific programatic tests I came to the conclusion that it is the sheer number of queries Umbraco BO is running to load the content, each with its own latency, that was causing the slowness rather than large expensive queries.

bielu commented 7 months ago

I spend some time debuging umbraco source code and this doesnt make sense for me. We are using IDataTypeService, which later speaks with DataTypeRepository which caches everything for 5 minutes. which means we should not see much delay because of speaking to IDataTypeService... So I noticed the cache works in some context and some not, I couldn't see any logic which would determine why it is not always cached when speaking with IDataTypeService.

bielu commented 7 months ago

So I had play around with umbraco source code and added mapperContext to method ToEditor. and there is comparison: Vanilla umbraco:

image

There is result by just including dictionary in ToEditor:

image

this is current results:

image

I removed almost 250 usages of datatype service, which removed over 2/3 of time. So when added custom profiling with decorator I finally started understanding what is happening, the Runtime cache is takes around 0.75ms to respond to query, a nd just because of volume we would add 150ms just to read from it, on top of it there are operation on each of datatypes which takes between 0.5 up to 2.5 ms per datatype so there is coming another 240ms! so it is just volume of request to runtime cache and on top overoperations which takes majority of processing time.

miguelcrpinto commented 7 months ago

@bielu do you know if V8 was also affected by this?

We have a V8.18.12 instance with way more nodes (I'd say more than 15k) and that often takes minutes to publish pages with lots of blocks in a block list property. In our case, even though we have quite a few languages enabled in the CMS, we are not using the language variants.

Loading such nodes in the backoffice also takes a while (nearly 1 minute just the GetById API call)

bielu commented 7 months ago

@miguelcrpinto yes v8 would be affect by this. My pr addresses this, but afaik if this PR will be merged I dont think so it will be backport to v8, especially as this pr is pretty intrusive to and use default interface implementations, which wouldn't work in v8 (but I might be wrong)

Migaroez commented 6 months ago

Hey @dawoe, have you had the chance to test your setup on 13.2.0-RC?

dawoe commented 6 months ago

@Migaroez I just tested it.

Our page with the most variants now loads in under 500ms.

erwindamsma commented 6 months ago

Will this solution also be released for v10?

Migaroez commented 6 months ago

@dawoe That is amazing news 🚀

@erwindamsma we are looking into it, we want to and it should be possible, there are just a few time constraints right now.

Might have an ETA next monday.

erwindamsma commented 6 months ago

Any update on the ETA?

erwindamsma commented 5 months ago

Hey @Migaroez,

Any idea if (and when) the fix will be available for Umbraco 10?

Migaroez commented 5 months ago

Hey @erwindamsma, sorry for the late reply, been busy and sick for a while. Still trying to catch up.

Our current stance is that we will not invest the time to backport this to v10/12 at the moment due to everyone being hands on with v14 and the upgrade path from v10 to v13 should be very straight forward.

gurumurthyjv commented 4 months ago

Hello All,

Does the back office slowness with multiple variants enabled node is fixed in v13 ? or is it planned for any features releases.

We are facing issue while saving & publishing, content with 24 variants in Umbraco v12.3.6, it almost takes 5min per save or publish, even it will be more in some cases.

Regards, Gurumurthy JV

dawoe commented 4 months ago

Hi @gurumurthyjv

This has been fixed in 13.2.0

bielu commented 4 months ago

@dawoe it is not fixed, it is mitigate :), it still slowing down but now it is less direct hit :)

gurumurthyjv commented 4 months ago

@dawoe ,

Do we have any timelines like when this will be fixed in v13 ?

Thanks @bielu , for responding 👍

Thanks, Gurumurthy JV

bielu commented 4 months ago

@gurumurthyjv there is no time line for this afaik, also just upgrading to 13.2 will resolve your issue, unless we speak about thousands of variants, where is issue still persist :)

gurumurthyjv commented 4 months ago

Hi @bielu and @dawoe ,

I have upgraded from v12 to v13.3, but still the save & publishing is taking more then 1min, which was not the case in v8 (less then 15sec). we have around 24 variants for each node.

Does this 1min for each save is desirable ?

Regards, Gurumurthy JV

bielu commented 4 months ago

Hey @gurumurthyjv, that sounds weird are you using any publish/save notifications? If so are you using data type service?

gurumurthyjv commented 4 months ago

Hi @bielu ,

Yes, we do have these notifications ContentPublishedNotification,SendingContentNotification and ContentSavedNotification for our custom requirement's.

bielu commented 4 months ago

@gurumurthyjv can you disable them and check if performance is better without? As than we know where to look 😉

gurumurthyjv commented 4 months ago

Hi @bielu ,

Thanks for your response, after disabling the content notifications, I don't see any differences. its the same.

bielu commented 4 months ago

@gurumurthyjv if you can create example project which will reproduce issue as you have than I can profile it and propose additional adjustments to speed up things :)

techcolin commented 3 months ago

We have just upgraded our v8 instance to v13.3 so we have some timings to share.

The worst page we have has 13 variants, and 65 blocks including nested components. Some of these have settings on them to control background colours and alignment etc. Appreciate this is an unusually large page, but it's something that was developed to client requirements at the time and not expected to suffer any issues.

We see some improvement, but not enough really, particularly around publishing.

Back-Office Page Load: U8 = 18.54 seconds U13 = 13.5 Improvement: 28%

Save single language: U8 = 80 seconds U13 = 43.5 seconds Improvement: 45%

Publish UK variant: U8 = 88 seconds U13 = 80 seconds Improvement: 10%

bielu commented 3 months ago

@techcolin as mentioned earlier I can spend time profiling things and make additional suggestions to HQ to improve things, but I need example solution which suffers biggers issue than mine as I managed to be only at 5s top post fix.

Also can you maybe share what infrastructure you have as that would help also determine what bottle neck you have.

techcolin commented 3 months ago

Both the v8 and v13 environments are running on Azure VMs - both D4as 4-core 16GB RAM with SSDs. SQL Azure elastic pool for the databases on 200 eDTUs, but under no load whatsoever.

Creating a test instance might be tricky as we're very busy at the moment. We could definitely supply uSync files if that helps at all?

bielu commented 3 months ago

Depends on what you mean by usync files, if you can include content in then they will deffo help, if just document types that they will help but much less

techcolin commented 3 months ago

We'll be able to provide what you need I think - either in usync files (doc types and content) or an environment for you to access, please just give us a few days as we've got sites launching this week.

erwindamsma commented 3 months ago

Hey @erwindamsma, sorry for the late reply, been busy and sick for a while. Still trying to catch up.

Our current stance is that we will not invest the time to backport this to v10/12 at the moment due to everyone being hands on with v14 and the upgrade path from v10 to v13 should be very straight forward.

We migrated the Umbraco 10 site to the latest v13, but just like @techcolin, not all has been resolved. We went from save times of approximately 60 seconds to around 34 to 38 seconds.

The same client also has some legacy sites (in the same Umbraco) using nested content instead of the blocklist editor. Those save within 1 tot 2 seconds, and sometimes even in less than a second.

techcolin commented 2 months ago

All.

We noted the release of 13.4 last week and have just upgraded our problem site to test on a staging server. https://our.umbraco.com/download/releases/1340

Happily, we can report on initial findings that our block-heavy page has improved from 26.8s load time for the main for GetById call, down to 3.98s

image image

This looks like the fix we were looking for. Obviously do your own testing, and note we haven't tested the upgraded version end-to-end, just had a look from this performance point of view.