Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prune Image Cropper and Media Picker (v3) values #11805

Merged
merged 5 commits into from
Jan 5, 2022

Conversation

ronaldbarendse
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Description

By default, Umbraco stores indented JSON values with redundant crop/default focal point values to the database for all Image Cropper and Media Picker (v3, with local crops) property data. You can view the existing data for both editors by doing the following database queries:

-- Get all Umbraco.ImageCropper property data
SELECT * FROM [umbracoPropertyData] WHERE propertyTypeId IN (
  SELECT id FROM cmsPropertyType INNER JOIN umbracoDataType ON cmsPropertyType.dataTypeId = umbracoDataType.nodeId AND umbracoDataType.propertyEditorAlias = 'Umbraco.ImageCropper'
);

-- Get all Umbraco.MediaPicker3 property data
SELECT * FROM [umbracoPropertyData] WHERE propertyTypeId IN (
  SELECT id FROM cmsPropertyType INNER JOIN umbracoDataType ON cmsPropertyType.dataTypeId = umbracoDataType.nodeId AND umbracoDataType.propertyEditorAlias = 'Umbraco.MediaPicker3'
);

On a clean Umbraco 8 install (v8/dev) with The Starter Kit, the image crop data on media is stored without indenting, but still contains an empty crops value (this is probably directly populated by the package install):

{"src":"/media/bqubxyxr/16403439029_f500be349b_o.jpg","crops":null}

After saving this media item in the back-office, it now contains indentation and also the default focal point, which both bloats this value from 67 characters to 138 (more than double):

{    "src": "/media/bqubxyxr/16403439029_f500be349b_o.jpg",    "focalPoint": {      "left": 0.5,      "top": 0.5    },    "crops": null  }

This gets even worse when you start configuring crops, as the crops are also stored in the property data, even when you've not changed the crop coordinates. After adding 2 crops to the Image Cropper data type (Avatar 100x100 and Hero 1920x200) and saving the media item again, the value is now 360 characters:

{    "src": "/media/bqubxyxr/16403439029_f500be349b_o.jpg",    "focalPoint": {      "left": 0.5,      "top": 0.5    },    "crops": [      {        "alias": "Avatar",        "width": 100,        "height": 100,        "coordinates": null      },      {        "alias": "Hero",        "width": 1920,        "height": 200,        "coordinates": null      }    ]  }

There's already code available to apply any missing configured crops to this value (added in #6950), so we don't have to store the crops when it doesn't have coordinates (the width/height is always redundant). The focal point is also optional and the code already deals with a missing value, so there's no need to store the default centered (0.5,0.5) value.

The same happens for the Media Picker (v3), because that also stores redundant values for the local crops and focal point. This PR updates both editors to prune the values in IDataValueEditor.FromEditor() and re-add the configured crops in the IDataValueEditor.ToEditor() method.


Make sure you've observed the above cases and then apply this PR. After saving the media item, the following data should be stored in the database:

{"src":"/media/bqubxyxr/16403439029_f500be349b_o.jpg"}

If you change the focal point and save, this should be the only thing that's added:

{"src":"/media/bqubxyxr/16403439029_f500be349b_o.jpg","focalPoint":{"left":0.64347202295552364,"top":0.24395730824424425}}

After resetting the focal point (which is added in PR #10923) and changing one of the crops, only the crop alias and coordinates are now stored on save:

{"src":"/media/bqubxyxr/16403439029_f500be349b_o.jpg","crops":[{"alias":"Avatar","coordinates":{"x1":0.70518867924528306,"y1":0,"x2":0,"y2":0.55792116850276741}}]}

The same can be tested for the Media Picker by enabling the local focal point and adding custom crops to the picker. Also make sure the pruned data is added back when rendering the value on the front-end, e.g. by including the following in the Home.cshtml view:

<pre>@Json.Encode(Model.HeroBackgroundImage.Value("umbracoFile"))</pre>

This returns a serialized version of the ImageCropperValue that's used in the GetCropUrl() method and should include all crops with their dimensions, e.g.:

{"Src":"/media/bqubxyxr/16403439029_f500be349b_o.jpg","FocalPoint":null,"Crops":[{"Alias":"Avatar","Width":100,"Height":100,"Coordinates":{"X1":0.70518867924528306,"Y1":0,"X2":0,"Y2":0.55792116850276741}},{"Alias":"Hero","Width":1920,"Height":200,"Coordinates":null}]}

@ronaldbarendse
Copy link
Contributor Author

ronaldbarendse commented Dec 30, 2021

Besides storing less data in the umbracoPropertyData table, NuCache also has to store less: both in the umbracoContentNu table, the NuCache.Content.db/NuCache.Media.db files and the in-memory cache (which AFAIK stores the raw data, as can be checked within the back-office by installing the Our.Umbraco.NuCacheExplorer package).

Because media doesn't have versions, just re-saving them from the back-office will prune the existing values. For content, re-saving would result in a new version with the now pruned values (which would actually increase the database size slightly). Data reduction will only happen after publishing the content, as that will update NuCache (although previewing should already update the draft version in the cache). You can use the Content Version Policy (introduced in PR #11495) to remove old versions, so you only store the latest, pruned values...


To see how much data can be saved, I've written scripts that prunes existing data: https://gist.github.com/ronaldbarendse/cda65cb041c149e943d4be67ed091da0.

I've taken a medium-sized production database that contained 1109 rows of ImageCropper property data (only on media). The values of all rows combined contained 963.852 characters, the umbracoPropertyData table reported 52.088 KB data space used and the cmsContentNu table 64.800 KB.

After running the prune script, the total characters went down from 963.852 to 72.505 characters, that's only 7,5% of the original size! 🤯 After rebuilding the tables and shrinking the database, the umbracoPropertyData table reported 51.560 KB data space used (528 KB less) and the cmsContentNu table 62.064 KB (2.736 KB less). This was using Umbraco.Web.PublishedCache.NuCache.Serializer = JSON, so I expect the size reduction of the cmsContentNu table to be less when MsgPack is used, but probably still significant...

All in all, this can quickly add up, especially when a site has a lot of media items and/or configured crops.

@ronaldbarendse
Copy link
Contributor Author

The Examine indexes will also benefit from this, as both the ExternalIndex and InternalIndex includes the raw property data for the Image Cropper and Media Picker (v3). We might want to look into doing similar improvements on different editors as well, especially those that store large amounts of JSON data, like the Block List, Nested Content and Grid editors.

Copy link
Member

@elit0451 elit0451 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great explanations, notes and reportings from the tests! 💪
After switching from to the PR branch, though, adding Hero and Avatar crops and saving a media item, all coordinates had null as values (see the image).
image

This also prevented the image to be displayed.
image

I also left a single code suggesiton

@ronaldbarendse
Copy link
Contributor Author

@elit0451 I can't replicate the issue with the coordinates having null values using a clean Umbraco install, adding the crops to the Image Cropper data type and saving media items. Without changing the focal point or any of the crops, only the src value is saved to the database and after changing the crops, the correct coordinates are saved (exactly as shown in my examples)...

@elit0451
Copy link
Member

elit0451 commented Jan 5, 2022

Hi again,
I tested again on a clean Umbraco install with the SK. Adding the crops to the Image Cropper data type and saving media items - saves correctly, as you pointed out but when I try changing the crops, I can no longer move the image (pick a specific area for what to crop) and after saving, the null values show up (when I observed that, I also tested with moving the focal point)

@elit0451
Copy link
Member

elit0451 commented Jan 5, 2022

Update ❗ After clearing the cache, the issue didn't persist, so I am happy to merge 😊

@elit0451 elit0451 merged commit 75bb805 into v8/dev Jan 5, 2022
@elit0451 elit0451 deleted the v8/feature/prune-imagecropper-mediapicker3-values branch January 5, 2022 10:11
ronaldbarendse added a commit that referenced this pull request Jan 25, 2022
bergmania pushed a commit that referenced this pull request Jan 26, 2022
* Apply changes from #11805 and #11806 to v9

* Update documentation and cleanup code styling
@ronaldbarendse ronaldbarendse added release/9.3.0 category/performance Fixes for performance (generally cpu or memory) fixes labels Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/performance Fixes for performance (generally cpu or memory) fixes release/8.18.0 release/9.3.0 type/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants