Refactor and improve the texture cache, implement new allocator. #1572

glennw · 2017-08-14T04:11:06Z

Refactor and improve the texture cache implementation.

All textures using the sColor* samplers are now array textures. This will allow us to simplify the large image tiling code in the future.
All texture updates are performed via PBO, to eliminate CPU driver stalls when updating textures.
The texture cache now acts more like a traditional cache, where the cache is a fixed size, and can evict items as required.
The allocator is switched to a slab style allocator, allowing deterministic behavior when allocating and freeing.
The slab allocator does not suffer from fragmentation like the previous allocator, and has no coalesce requirements.
The eviction policy is a lot smarter than the previous implementation. Items are evicted from the cache as required.
Since the shared cache is a fixed / bounded size, any allocations that are too large for the cache are created as standalone textures.
In the rare / edge case of the fixed sized cache being full, and the cache not being able to evict items, these items also fallback to the standalone texture allocator.
Add support for array textures that don't have FBOs.
Simplify render targets (always array textures).
Remove resize_texture which is now unused.
Remove support for texture upload without PBO.
Add support for weak handles to freelist.
Update gleam to 0.4.8 (allows upload of array textures via PBO).

This change is


        Update gleam to 0.4.8 (allows upload of array textures via PBO).


        Remove coalesce benchmark that is no longer relevant.


        Add support for weak handles to freelist.


        Add support for array textures that don't have FBOs.

Simplify render targets (always array textures). Remove resize_texture which is now unused. Support uploading all texture types via PBO. Remove support for texture upload without PBO.


        Switch debug renderer to use array textures.


        Update shaders that fetch from sColor* to use array samplers.


        Refactor and improve the texture cache implementation.

* The texture cache now acts more like a traditional cache, where the cache is a fixed size, and can evict items as required. * The allocator is switched to a slab style allocator, allowing deterministic behavior when allocating and freeing. * The slab allocator does not suffer from fragmentation like the previous allocator, and has no coalesce requirements. * The eviction policy is a lot smarter than the previous implementation. Items are evicted from the cache as required, and not before. * Since the shared cache is a fixed / bounded size, any allocations that are too large for the cache are created as standalone textures. * In the rare / edge case of the fixed sized cache being full, and the cache not being able to evict items, these items also fallback to the standalone texture allocator. * All textures using the sColor* samplers are now array textures. This will allow us to simplify the large image tiling code in the future. * All texture updates are performed via PBO, to eliminate CPU driver stalls when updating textures.

glennw · 2017-08-14T04:11:22Z

r? @kvark

I'm so sorry for the size of this patch. It really snowballed. Let me know if you want to discuss details on IRC / Vidyo etc.

I'm happy with the end result though :)

I've done a fair amount of manual testing in both Servo and Gecko. It also passes the Gecko sanity reftests, the Servo WPT/CSS tests and the Wrench reftests.

glennw · 2017-08-14T04:11:47Z

@staktrace @sotaroikeda @JerryShih I've run the gecko sanity tests locally with this, and they pass. It doesn't change any external interfaces, but it does expect that external textures which use the normal texture2D sampler switch over to place them into the texture2DArray sampler. It doesn't affect anything that goes via the TextureRect or TextureExternal image types. Do you foresee any issues with this? I'll ping you later during the week with some questions I have on running a try build with this change.

glennw · 2017-08-14T04:15:16Z

@kvark PS: The texture_cache.rs file is different enough now that it probably is easier to just review it as a new file rather than trying to diff it...

glennw · 2017-08-14T04:24:50Z

To get a feel for how the cache works, the image below may help. This is what the cache looks like after browsing a heap of wikipedia and mozilla pages in Servo.

You can see that within each layer of a texture array, there are number of "regions". Each region acts as a slab allocator with a fixed size, that allows fast and deterministic allocating and freeing.

We'll probably want to tweak the slab sizes, but the initial implementation seems to work well on all the pages I've tried so far.


        Fix shader validation error.

kvark · 2017-08-14T18:29:39Z

Overall, I think moving to slab allocation is a great step forward! The amount of code is making it rather difficult to get a full picture even when using Reviewable, but I got a general idea, and I like it (despite the fact I was cleaning up the coalescing not so long ago) ;)

In the rare / edge case of the fixed sized cache being full, and the cache not being able to evict items, these items also fallback to the standalone texture allocator.

Hmm, not sure about this one. I feel like we should not be hard-coding the texture cache size. If there is a workload that needs a little more than we have, we'll see consistent drop in performance there, where optimally I'd like our performance curve to be smooth.

So, instead of allocating a standalone texture, can we just allocate a new texture page to be added to the cache (and reused later on)?

To get a feel for how the cache works, the image below may help.

I love those, thank you! Although, perhaps it would be visually more appealing to render them Y-flipped, so that all the textures look exactly as they are on screen.

Reviewed 32 of 34 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 17 unresolved discussions.

webrender/res/debug_font.fs.glsl, line 10 at r2 (raw file):

void main(void)
{
#ifdef SERVO_ES2

is that check intended for removal?

webrender/src/device.rs, line 1644 at r2 (raw file):

        let (gl_format, bpp, data, data_type) = match self.textures.get(&texture_id).unwrap().format {
            ImageFormat::A8 => {
                if cfg!(any(target_arch="arm", target_arch="aarch64")) {

is this path intended for removal as well?

webrender/src/freelist.rs, line 94 at r2 (raw file):

    }

    pub fn upsert(&mut self,

please add a comment explaining what upsert means in this context

webrender/src/glyph_cache.rs, line 14 at r2 (raw file):

    pub texture_cache_handle: TextureCacheHandle,
    pub glyph_bytes: Arc<Vec<u8>>,
    pub width: u32,

should it use euclid primitives, perhaps? I.e. TypedSize<u32> and TypedPoint2D<f32>

webrender/src/glyph_rasterizer.rs, line 161 at r2 (raw file):

            match glyph_key_cache.entry(key.clone()) {
                Entry::Occupied(mut entry) => {
                    if let &mut Some(ref mut glyph_info) = entry.get_mut() {

nit: probably easier to have if let Some(...) = *entry.get_mut()

webrender/src/glyph_rasterizer.rs, line 273 at r2 (raw file):

                    let glyph_bytes = Arc::new(glyph.bytes);
                    let mut texture_cache_handle = TextureCacheHandle::new();
                    texture_cache.request(&mut texture_cache_handle, gpu_cache);

shouldn't we check for the request return value?

webrender/src/glyph_rasterizer.rs, line 274 at r2 (raw file):

                    let mut texture_cache_handle = TextureCacheHandle::new();
                    texture_cache.request(&mut texture_cache_handle, gpu_cache);
                    texture_cache.update(

is update always following request?

webrender/src/internal_types.rs, line 100 at r2 (raw file):

#[derive(Copy, Clone, Debug, PartialEq)]
pub enum RenderTargetMode {

I feel like we'll eventually (not now!) need to have a separate TargetView type (instead of this enumeration) that would capture the internal FBOs (per slice) and the depth attachment.

webrender/src/renderer.rs, line 2417 at r2 (raw file):

        let mut size = 512;
        let fb_width = framebuffer_size.width as i32;
        let num_textures: i32 = self.cache_texture_id_map

shouldn't this be num_layers?

webrender/src/resource_cache.rs, line 108 at r2 (raw file):

}

pub struct ResourceClassCache<K,V> {

is there any point in this struct now as oppose to just typedeffing to FastHashMap?

webrender/src/resource_cache.rs, line 144 at r2 (raw file):

    where for<'r> F: Fn(&'r &K) -> bool
    {
        let resources_to_destroy = self.resources.keys()

I think it can be easily done on the caller site now that we have Rust-1.19 (used by Gecko) and all those pretty retain methods in libstd

webrender/src/texture_cache.rs, line 82 at r2 (raw file):

// cache or a standalone texture.
#[derive(Debug)]
struct CacheEntry {

nice struct!

webrender/src/texture_cache.rs, line 231 at r2 (raw file):

    // Request an item in the texture cache. All images that will
    // be used on a frame *must* have request() called on their

do we have any detection of a forgotten request?

webrender/src/texture_cache.rs, line 404 at r2 (raw file):

        // Sort by access time so we remove the oldest ones first.
        eviction_candidates.sort_by_key(|handle| {

I think this procedure can be done simpler and a bit faster without temporary arrays:

sort standalone_entry_handles in descending order of last_access. Given that it's likely to be temporarily coherent, a quicksort-like method would be efficient at skipping the work on already sorted parts
check 32th from the back for early return
pop 32 items, calling free() for each

Although, this doesn't seem to be a performance critical place here, so we can look into it later. The current code is correct.

webrender/src/texture_cache.rs, line 477 at r2 (raw file):

                let entry = self.entries.free(handle);
                if let Some(region) = self.free(entry) {
                    found_matching_slab = found_matching_slab ||

can we use |= operator?

webrender/src/texture_cache.rs, line 870 at r2 (raw file):

        // allocate from.
        let slab_size = SlabSize::new(width, height);
        let slab_size_bytes = slab_size.get_size();

I thought that get_size() returns the width of a slab region, so it seems confusing to see _bytes here

webrender/src/texture_cache.rs, line 906 at r2 (raw file):

                region.init(slab_size);
                if let Some(location) = region.alloc() {
                    entry_kind = Some(EntryKind::Cache {

nit: could be entry_kind = region.alloc().map(|location| {...});

Comments from Reviewable


        Address review comments.

glennw · 2017-08-14T21:54:02Z

Review status: 29 of 34 files reviewed at latest revision, 17 unresolved discussions.

webrender/res/debug_font.fs.glsl, line 10 at r2 (raw file):