[WPE 2.38] Thumbnails scrolling isn't as smooth as for WPE 2.22 in VTM Go app

jakub-gocol-red commented 11 months ago

Reproduction steps

Open VTM Go app (https://stv.vtmgo.be/vtmgo/prerelease/eos/index.html#/main/storefront?type=main)
Scroll thumbnails left/right

Expected behaviour

Thumbnails scrolling is as smooth as on WPE2.22

Actual behaviour

Thumbnails scrolling isn't smooth. First div border is moved to next thumbnail, after delay thumbnails are moved. Animation sometimes "jumps" quicker, than expected.

Notes

Issue reproduces on Sagemcom Video Accelerator (Sagecom m393 with Broadcom 72180 chip). Video comparing WPE2.22 (left) and WPE2.38 (right)

https://github.com/WebPlatformForEmbedded/WPEWebKit/assets/93707234/7376b6ec-2e56-4cbe-b4a6-8efc13f1b1d0

rychem commented 9 months ago

@modeveci would you share update please whether you see the issue from your side?

rychem commented 9 months ago

@magomez the issue is reproducable on Video Accelerator platform - did you maybe have chance to try to reproduce?

magomez commented 9 months ago

I have this on my TODO list, but I haven't had time to look into it, as I'm dealing with more critical issues. I'll work on it as soon as I have some time.

tomasz-suchon-red commented 9 months ago

Issue can be also reproducible on Sagemcom Video Accelerator with WPE 2.28 imagename:m393-72180ZB-stb_FBT_rdk-next_20230211001029

rychem commented 8 months ago

Hi @magomez, is there any update for that issue ... ?

rychem commented 7 months ago

@magomez , @woutermeek we are trying to understand Igalia/Metrological perception of this performance degradation vs. WPE2.22, however for longer time we are sending kind request for update without response ... Can I kindly ask for any feedback?

magomez commented 7 months ago

@jakub-gocol-red is this on 720p or 1080p?

I've been looking a bit into what the page does. My god there are ton of layers there. I see that the devs use a lot the will-change property to indicate that something is going to be animated and it should get its own buffer. but I also see that it's being used in some places where it's not needed and missing in places where it's needed, like the divs that are scaled up/down when selected/deselected.

I'll keep digging, see what I can find.

magomez commented 7 months ago

@jakub-gocol-red is this on 720p or 1080p?

Also, so you have threaded rendering enabled? If you're using WPEFramework, this is enabled through the threadedpainting property in the WebKitBrowser plugin configuration. You can indicate there the number of threads used for rendering. This is useful if the device has several cores. If the device is quad core, you can set that to 2 or 3. If it's just a dual core there's no really benefit from enabling it. If you're not using WPEFramework, the number of threads can be set using the WEBKIT_NICOSIA_PAINTING_THREADS env var.

I've been looking a bit into what the page does. My god there are ton of layers there. I see that the devs use a lot the will-change property to indicate that something is going to be animated and it should get its own buffer. but I also see that it's being used in some places where it's not needed and missing in places where it's needed, like the divs that are scaled up/down when selected/deselected.

Another thing: the images used for the covers are 320x480, and they are painted with a size of 228x342 which means that they need to be rescaled, which means more time wasted during the rendering.

jakub-gocol-red commented 7 months ago

@magomez It's on 1080p, threadedpainting set to 1 and device has 4 cores. It's the same hardware and mentioned settings are the same on both screens from the video in first comment. Page itself might not be written very well, but performance was acceptable on 2.22. The only difference is browser itself.

jakub-gocol-red commented 7 months ago

and about painting threads, setting it to 4 improves situation a bit, but it's nowhere close to 2.22. It's still slow

magomez commented 7 months ago

I finally understood what's happening here.

Short version: The MemoryPressure configuration limits that are being set are too small to be able to run the app smoothly. The app consumes quite a lot of memory, which goes beyond the limit where the browser is allowed to accelerate animations by creating intermediate buffers. So, those intermediate buffers are not created, animations are not accelerated, and the animation performance is horrible. You can check that this is the problem by removing the memory limits. These limits are set by the WPEFramework, and set in the configuration file /etc/WPEFramework/plugins/WebKitBrowser.json in the device. There should be the parameter "webprocesslimit":300, that sets the max memory usage of the process to 300MB. Just remove it and the performance will improve.

Long version: The MemoryPressureSettings API allows defining how the browser works depending on how much memory is being used, so it can adapt somehow and try to stay below a defined limit. This limit is set per process, and that's what is defined in that WPEFramework config value (let's use the 300MB limit as an example). But besides that limit, there are other very important settings to handle, which are the fractions. There are 3 fractions:

Conservative threshold fraction: indicates a percentage of the memory limit where the browser will have a conservative behavior regarding increasing memory usage, will try to free memory more often, and the performance will degraded. By default this value is 0.33, which means that it's 99MB for the 300MB we defined as the limit. Below the conservative limit, the browser will use memory without restrictions, prioritizing performance.
Strict threshold fraction: indicates a percentage of the memory limit where the browser will have a strict behavior regarding increasing memory usage, and will try to aggressively free memory more often, hurting the performance a lot. By default this value is 0.5, which means that it's 150MB for the 300MB we defined as the limit.
Kill threshold fraction: indicates the percentage of the limit where the process will be killed for excessive memory usage. By default this is not used.

So, what happens when the app is run? During the rendering, new buffers are being created to accelerate the animations, but the page has tons of layers and also it's FHD, so the 99MB memory limit is reached quite quickly. This puts the browser in conservative mode, where it tries to save memory by not creating buffers for the animations, so the animations are not accelerated and the performance is quite bad.

Removing the memory limit fixes it because the browser is never into conservative mode and the animations are accelerated. In this case I've seen that the memory consumption of the page is below 200MB so by setting the limit to 600MB the conservative limit will be ~200MB, enough for not entering the conservative mode and keeping the animations accelerated.

But the real fix for this is allowing WPEFramework to set the conservative and strict fractions, something that's not possible now. The default values of 0.33 and 0.5 are too low when we're also setting a low limit. For a limit of 300MB, probably something like 0.7 and 0.9 would make more sense, so the browser prioritizes performance until those thresholds are achieved.

@modeveci this requires some implemenetation on the WebKitBrowser plugin side, as it only allows setting the limit and the poll interval nowadays.

magomez commented 7 months ago

PS: I can also modify the default values for those fractions in WPE, but this wouldn't be an ideal fix, because the fractions need to be defined related to the limit set. 0.7 as the conservative value makes sense with a limit of 300MB for example, as the browser has 210MB of memory to use before hurting the performance. But it can be too low for a limit of 100MB for example. The values should also depend on the amount of memory of the device and whether it's used for 720p or 1080p, for example, as this changes the memory usage a lot.

woutermeek commented 7 months ago

@magomez @modeveci We should make these settings configurable, it is nice to give some apps a little more breathing room.

magomez commented 7 months ago

In the meanwhile I'm going to change the default values to something that makes more sense, like 0.8 and 0.9 for example. This should directly fix this issue, and give us more time to work on the new configurable parameters.

modeveci commented 7 months ago

I agree with Wouter, we need to make it available to configure via plugin browser. I will work on that! And It is good to have default values from browser more relax than strict. @magomez Thanks!

magomez commented 7 months ago

I've just pushed 97a554d97bee1fdb1ba21a34162f3129aba3d72e to the wpe-2.38 branch, updating the default values to 0.8 and 0.9, so this should not be reproducible anymore. I'll port that change to 2.42 as well.

magomez commented 7 months ago

Do we have any feedback about this? Is the problem solved and the issue can be closed?

amol-virnodkar-infosys commented 6 months ago

@magomez We are testing this patch and we will provide an update on the testing soon.

amol-virnodkar-infosys commented 6 months ago

Using this patch - https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/97a554d97bee1fdb1ba21a34162f3129aba3d72e, the issue is not reproducible now. Thanks.

magomez commented 6 months ago

Using this patch - 97a554d, the issue is not reproducible now. Thanks.

Great, closing this then. Thanks!

WebPlatformForEmbedded / WPEWebKit