UI: Increase File Browser Minimum Cache size #107711

Harley Acheson · 2023-05-07T19:53:31+02:00

Harley Acheson commented

2023-05-07 19:53:31 +02:00

Increase minimum size of the sliding window cache size from 256 to 1024

The File Browser keeps a sliding window cache of listed items. Currently this cache starts with a size of 1024, set in FILELIST_ENTRYCACHESIZE_DEFAULT. In all cases while viewing file system items in any thumbnail view this size is reduced to 256. This means that when viewing any listing of more than 256 items the previews are discarded and need to be re-read from the cached thumbnail files. This leads to substantially decreased performance for a list size that is not unusually large.

The purpose of filelist_file_cache_slidingwindow_set is to set the cache size approximately twice the size of the number of files shown. For list views this calculated value is typically 128 and so the cache size becomes 256. But for thumbnail views the value sent into this function is in the range of 12-48 and we end up using the current minimum of 256. Note that for Asset lists the value passed is 256 and we use a cache size of 512. This PR just makes the minimum be 1024.

Note that the description above of the previews being re-read from cached thumbnail files is currently not working, but will be fixed in #107698. Currently the files that are outside of the cache size continue to display generic icons.

Increase minimum size of the sliding window cache size from 256 to 1024 --- The File Browser keeps a sliding window cache of listed items. Currently this cache starts with a size of 1024, set in FILELIST_ENTRYCACHESIZE_DEFAULT. In all cases while viewing file system items in any thumbnail view this size is reduced to 256. This means that when viewing any listing of more than 256 items the previews are discarded and need to be re-read from the cached thumbnail files. This leads to substantially decreased performance for a list size that is not unusually large. The purpose of `filelist_file_cache_slidingwindow_set` is to set the cache size approximately twice the size of the number of files shown. For list views this calculated value is typically 128 and so the cache size becomes 256. But for thumbnail views the value sent into this function is in the range of 12-48 and we end up using the current minimum of 256. Note that for Asset lists the value passed is 256 and we use a cache size of 512. This PR just makes the minimum be 1024. Note that the description above of the previews being re-read from cached thumbnail files is currently not working, but will be fixed in #107698. Currently the files that are outside of the cache size continue to display generic icons.

Harley Acheson added 1 commit 2023-05-07 19:53:38 +02:00

a94d6e6fd6 UI: Increase File Browser Minimum Cache size

Increase minimum size of the sliding window cache size from 256 to 1024

Harley Acheson added this to the User Interface project 2023-05-07 19:53:47 +02:00

Harley Acheson requested review from Julian Eisel 2023-05-07 19:53:55 +02:00

Julian Eisel commented

2023-05-08 17:10:23 +02:00

I'm not sure about this, this kind of defeats the purpose of having the cache at all. Of course previews will be more readily available while scrolling the more items we keep in memory and that feels snappier, but the point of this sliding window cache is to not have too much unviewed data in memory. In a release build and with an SSD I barely notice that the previews are being reloaded (debug builds are quite a bit slower here).

Using FILELIST_ENTRYCACHESIZE_DEFAULT in filelist_file_cache_slidingwindow_set() makes sense, but I would update that to be 256 since that is the de facto default value it seems.

@mont29 your opinion?

I'm not sure about this, this kind of defeats the purpose of having the cache at all. Of course previews will be more readily available while scrolling the more items we keep in memory and that feels snappier, but the point of this sliding window cache is to not have too much unviewed data in memory. In a release build and with an SSD I barely notice that the previews are being reloaded (debug builds are quite a bit slower here). Using `FILELIST_ENTRYCACHESIZE_DEFAULT` in `filelist_file_cache_slidingwindow_set()` makes sense, but I would update that to be 256 since that is the de facto default value it seems. @mont29 your opinion?

Harley Acheson commented

2023-05-08 17:13:27 +02:00

@JulianEisel Could consider a compromise of 512 for FILELIST_ENTRYCACHESIZE_DEFAULT? That is the value used currently for Asset lists (256 passed to filelist_file_cache_slidingwindow_set() in AssetList::ensurePreviewsJob).

Julian Eisel commented

2023-05-08 17:23:48 +02:00

As a sidenote, I don't think the whole caching adds much value currently, other than the sliding window for previews. FileDirEntry is basically a duplicate of FileListInternEntry plus the preview. AFAIK the caching was added in preparation of the old asset manager design.

General UI code now supports asynchronous loading of previews too (see PreviewLoadJob). #107576 already uses this, and performs a lot better with large asset libraries. UI code could also garbage collect unused previews after some time.

As a sidenote, I don't think the whole caching adds much value currently, other than the sliding window for previews. `FileDirEntry` is basically a duplicate of `FileListInternEntry` plus the preview. AFAIK the caching was added in preparation of the old asset manager design. General UI code now supports asynchronous loading of previews too (see `PreviewLoadJob`). #107576 already uses this, and performs a lot better with large asset libraries. UI code could also garbage collect unused previews after some time.

Julian Eisel commented

2023-05-08 17:41:32 +02:00

@JulianEisel Could consider a compromise of 512 for FILELIST_ENTRYCACHESIZE_DEFAULT? That is the value used currently for Asset lists (256 passed to filelist_file_cache_slidingwindow_set() in AssetList::ensurePreviewsJob).

Honestly, I'm not sure if that's worth it really. Quick calculation, 512 previews at a size of 256x256 would add 134MB of memory, half of that is already a lot for UI resources. Most of these previews don't matter at all while people are just working with the asset browser open. Is it worth using that much extra memory, just for a slightly nicer scrolling experience? I'd rather use time based garbage collection as mentioned.
Also, keep in mind that we may want to increase preview size at some point (e.g. from 256 to 512).

> @JulianEisel Could consider a compromise of 512 for FILELIST_ENTRYCACHESIZE_DEFAULT? That is the value used currently for Asset lists (256 passed to filelist_file_cache_slidingwindow_set() in AssetList::ensurePreviewsJob). Honestly, I'm not sure if that's worth it really. Quick calculation, 512 previews at a size of 256x256 would add 134MB of memory, half of that is already a lot for UI resources. Most of these previews don't matter at all while people are just working with the asset browser open. Is it worth using that much extra memory, just for a slightly nicer scrolling experience? I'd rather use time based garbage collection as mentioned. Also, keep in mind that we may want to increase preview size at some point (e.g. from 256 to 512).

Harley Acheson commented

2023-05-08 17:49:21 +02:00

@JulianEisel - Honestly, I'm not sure if that's worth it really

No worries, let's leave this for now. The bad scrolling experience really isn't that terrible. It can seem worse for us because it is much slower in debug mode. It is less noticeable for most users in release. I just wanted us to consider this while I was in here for #106023.

use that memory for when we want to increase preview size at some point (e.g. from 256 to 512).

If we move the thumbnail cache (#107298) so we use it exclusively we could could change from PNG to EXR - get a decrease in file sizes and increases in both reading and writing speed. We could then consider having the cache hold those in memory rather than uncompressed bitmaps, only converting when actually drawing or within a window. Could conceivably have previews at 1024

> @JulianEisel - Honestly, I'm not sure if that's worth it really No worries, let's leave this for now. The bad scrolling experience really isn't that terrible. It can seem worse **for us** because it is much slower in debug mode. It is less noticeable for most users in release. I just wanted us to consider this while I was in here for #106023. > use that memory for when we want to increase preview size at some point (e.g. from 256 to 512). If we move the thumbnail cache (#107298) so we use it exclusively we could could change from PNG to EXR - get a decrease in file sizes and increases in both reading and writing speed. We could then consider having the cache hold those in memory rather than uncompressed bitmaps, only converting when actually drawing or within a window. Could conceivably have previews at 1024

Harley Acheson closed this pull request

2023-05-08 19:48:06 +02:00

Harley Acheson deleted branch CacheSize

2023-05-08 19:48:14 +02:00

Julian Eisel referenced this issue from a commit

2023-05-08 20:01:20 +02:00

Asset Views: Decrease default preview cache size

Julian Eisel commented

2023-05-08 20:02:08 +02:00

Committed 50d49e24c4 to decrease the effective cache size for asset lists to 256.

Committed 50d49e24c4 to decrease the effective cache size for asset lists to 256.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

UI: Increase File Browser Minimum Cache size #107711

Pull request closed