The Pool lease efficiency

[qianzhang5]

I like your resource management by the pool-lease mechanism. But in some cases, it yields very low utility efficiency. because it uses the size in a hash key, in a very dynamic size changing situation, a leased entry gets reused chances very low. such as the modified your example below
I suggest you would not use actual size, replace with black size to increase reusing chance.

use {screen_13::prelude::, screen_13_egui::prelude::};

fn main() -> Result<(), DisplayError> {
    let event_loop = EventLoop::new()
        .desired_swapchain_image_count(2)
        .window(|window| window.with_transparent(false))
        .build()?;
    let mut egui = Egui::new(&event_loop.device, event_loop.as_ref());

    let mut cache = LazyPool::new(&event_loop.device);
    let mut count = 0;
    event_loop.run(|frame| {
        let img = frame.render_graph.bind_node(
            cache
                .lease(ImageInfo::new_2d(
                    vk::Format::R8G8B8A8_UNORM,
                    100,
                    100,
                    vk::ImageUsageFlags::SAMPLED | vk::ImageUsageFlags::TRANSFER_DST,
                ))
                .unwrap(),
        );
        frame
            .render_graph
            .clear_color_image_value(img, [0., 1., 0., 1.]);
        frame
            .render_graph
            .clear_color_image_value(frame.swapchain_image, [0., 0., 0., 1.]);

        let id = egui.register_texture(img);

        egui.run(
            frame.window,
            frame.events,
            frame.swapchain_image,
            frame.render_graph,
            |ui| {
                egui::Window::new("Test")
                    .resizable(true)
                    .vscroll(true)
                    .default_size([400., 400.])
                    .show(ui, |ui| {
                        ui.add(egui::Button::new("Test"));
                        ui.add(egui::Link::new("Test"));
                        ui.add(egui::Label::new(&format!("count:{}", "a".repeat(count) )));
                    });
            },
        );
        count += 1;
    })
}

and your hash.rs

impl Pool<BufferInfo, Buffer> for HashPool {
    fn lease(&mut self, info: BufferInfo) -> Result<Lease, DriverError> {
        let buffer_num = self.buffer_cache.len();
        let cache = self.buffer_cache.entry(info.clone())
            .or_insert_with(|| {
                Arc::new(Mutex::new(VecDeque::new()))
            });
        let cache_ref = Arc::downgrade(cache);
        let mut cache = cache.lock();

        if cache.is_empty() {
            let item = Buffer::create(&self.device, info)?;
            println!("created new item, cache size:{buffer_num}");
            return Ok(Lease {
                cache: Some(cache_ref),
                item: Some(item),
            });
        }

        Ok(Lease {
            cache: Some(cache_ref),
            item: cache.pop_front(),
        })
    }
}

impl Pool<BufferInfoBuilder, Buffer> for HashPool {
    fn lease(&mut self, builder: BufferInfoBuilder) -> Result<Lease, DriverError> {
        let info = builder.build();

        self.lease(info)
    }
}

// Enable leasing items using their basic info
macro_rules! lease {
    ($info:ident => $item:ident) => {
        paste::paste! {
            impl Pool<$info, $item> for HashPool {
                fn lease(&mut self, info: $info) -> Result<Lease<$item>, DriverError> {
                    let cache = self.[<$item:snake _cache>].entry(info.clone())
                        .or_insert_with(|| {
                            Arc::new(Mutex::new(VecDeque::new()))
                        });
                    let cache_ref = Arc::downgrade(cache);
                    let mut cache = cache.lock();

                    if cache.is_empty() {
                        let item = $item::create(&self.device, info)?;
                        return Ok(Lease {
                            cache: Some(cache_ref),
                            item: Some(item),
                        });
                    }

                    Ok(Lease {
                        cache: Some(cache_ref),
                        item: cache.pop_front(),
                    })
                }
            }
        }
    };
}

lease!(RenderPassInfo => RenderPass);
lease!(DescriptorPoolInfo => DescriptorPool);

// Enable leasing items as above, but also using their info builder type for convenience
macro_rules! lease_builder {
    ($info:ident => $item:ident) => {
        lease!($info => $item);

        paste::paste! {
            impl Pool<[<$info Builder>], $item> for HashPool {
                fn lease(&mut self, builder: [<$info Builder>]) -> Result<Lease<$item>, DriverError> {
                    let info = builder.build();

                    self.lease(info)
                }
            }
        }
    };
}

lease_builder!(AccelerationStructureInfo => AccelerationStructure);
//lease_builder!(BufferInfo => Buffer);
lease_builder!(ImageInfo => Image);

[qianzhang5]

The example shows a character adding, and the vertex buffer is changing, so the buffer cache never gets reused.

[attackgoat]

I don't understand your example. The code does not show any vertex buffer. I see a single 100x100 image being leased from a LazyPool and the implementation of HashPool.

Screen 13 currently has two different and interchangeable implementations of the pool logic:

• HashPool all resource keys are hashed and exact matches are returned only
• LazyPool has logic to return compatible resources that exceed the requirements but are available in the cache

Generally I would suggest always using LazyPool unless you have a specific situation where all resources will have the same keys. The above example from screen-13-egui would probably be better served using HashPool.

I think in a few places I also round-up the number of descriptors or buffer size in order to increase cache efficiency. See here.

Also, the code for leasing requires some additional work for clearing out or managing resources that has not been done yet. I don't feel like it's 1.0 yet and would welcome any PRs or improvements you might have - or better documentation.

[qianzhang5]

I am sorry for not explaining the example code. In the example code I added a text label, that character length increases by one in a loop. ### ui.add(egui::Label::new(&format!("count:{}", "a".repeat(count) ))); In hashpool code, I added a println to show the cache buffer size. ### println!("created new item, cache **size:{buffer_num}"); The example would show you there is no buffer in cache pool is reused (0% utilizing rate).

…

On Sat, Mar 11, 2023, 5:25 AM John Wells ***@***.***> wrote: I don't understand your example. The code does not show any vertex buffer. I see a single 100x100 image being leased from a LazyPool and the implementation of HashPool. Screen 13 currently has two different and interchangeable implementations of the pool logic: - HashPool all resource keys are hashed and exact matches are returned only - LazyPool has logic to return compatible resources that exceed the requirements but are available in the cache Generally I would suggest always using LazyPool unless you have a specific situation where all resources will have the same keys. The above example from screen-13-egui would probably be better served using HashPool . I think in a few places I also round-up the number of descriptors or buffer size in order to increase cache efficiency. See here <https://github.com/attackgoat/screen-13/blob/master/src/graph/resolver.rs#L762> . Also, the code for leasing requires some additional work for clearing out or managing resources that has not been done yet. I don't feel like it's 1.0 yet and would welcome any PRs or improvements you might have - or better documentation. — Reply to this email directly, view it on GitHub <#58 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADCC3ACASJ63EFX3QPVXOXLW3R4NDANCNFSM6AAAAAAVW67G64> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[attackgoat]

Oh - I understand now. I see that you're looking at the HashPool which is owned by the Egui struct; and you are correct the given pattern will not pick any buffers from the cache here.

I think a reasonable fix would be to change Egui to use a LazyPool and align the egui-returned vertices to a power of two or 8KB blocks. This would increase the pool utilization at the expense of memory.

screen-13-egui was written when Screen 13 was new and hasn't seen the need for many fixes, but these kinds of changes would be welcome.

[qianzhang5]

I propose a change in buffer.rs to increase cache utilization rate without changing egui/(user end) code. It benefits general buffer cases.
buffer.rs.zip

[attackgoat]

I think this feedback would be better suited as a PR review, so I could annotate code, but I can give a quick summary here. Also I would like to get you credit as the author with any changes :)

• Code which uses HashPool and leases buffers in some range (e.g. varying sizes but less than 2,048 bytes) will return a buffer whose BufferInfo hashes within the block size but whose actual size may be less than requested
  • For example:

    let buf = pool.lease(BufferInfo::new(1440u64, vk::BufferFlags::empty()))?;
    // buf.info.size is between 0 and 2048
    

• This could be resolved in Buffer::create by using the block size instead of actual size
• HashPool currently returns an exact match for a resource; so Buffer::mapped_slice_mut returns an exact-sized slice. Code which uses LazyPool must sub-slice the returned slice to ensure the actual bounds are used. With these changes HashPool results would also need to be sub-sliced.
• The 2KB choice is an opinion that I would rather not see hard-coded - perhaps it could be a field of BufferInfo which defaults to 1.

For screen-13-egui and other users of Screen 13 I think the best way to get the same effect is to round-up the size used to acquire a lease at the place where it is actually leased; because the end application knows best what block size is reasonable for the situation.

This change could also be done as a constructor, like BufferInfo::new_block_size or a helper function like BufferInfo::new(1440u64, vk::BufferFlags::empty()).block_size(2048u64).