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

[ll] Memory management - Heaps #1484

Closed
Bastacyclop opened this issue Sep 10, 2017 · 5 comments
Closed

[ll] Memory management - Heaps #1484

Bastacyclop opened this issue Sep 10, 2017 · 5 comments

Comments

@Bastacyclop
Copy link
Contributor

Bastacyclop commented Sep 10, 2017

This discussion is about memory management for render, necessary for #1466.
More precisely, heap management for images and buffers.

Quick reminder of the old memory API:

pub enum Usage {
    /// Full speed GPU access.
    /// Optimal for render targets and resourced memory.
    Data,
    /// CPU to GPU data flow with update commands.
    /// Used for dynamic buffer data, typically constant buffers.
    Dynamic,
    /// CPU to GPU data flow with mapping.
    /// Used for staging for upload to GPU.
    Upload,
    /// GPU to CPU data flow with mapping.
    /// Used for staging for download from GPU.
    Download,
}

pub flags Bind: u8 {
    /// Can be rendered into.
    const RENDER_TARGET    = 0x1,
    /// Can serve as a depth/stencil target.
    const DEPTH_STENCIL    = 0x2,
    /// Can be bound to the shader for reading.
    const SHADER_RESOURCE  = 0x4,
    /// Can be bound to the shader for writing.
    const UNORDERED_ACCESS = 0x8,
    /// Can be transfered from.
    const TRANSFER_SRC     = 0x10,
    /// Can be transfered into.
    const TRANSFER_DST     = 0x20,
}

As previously discussed, the Dynamic usage is not satisfying. We want to directly support update commands for Data instead (see #1418). We could remove it, or it could get another meaning (details following).

Memory type selection

First, we have to select what's best from the available memory types. The core API exposes the following properties:

pub flags HeapProperties: u16 {
    /// Device local heaps are located on the GPU.
    const DEVICE_LOCAL   = 0x1,

    /// CPU-GPU coherent.
    ///
    /// Non-coherent heaps require explicit flushing.
    const COHERENT     = 0x2,

    /// Host visible heaps can be accessed by the CPU.
    ///
    /// Backends must provide at least one cpu visible heap.
    const CPU_VISIBLE   = 0x4,

    /// Cached memory by the CPU
    const CPU_CACHED = 0x8,

    /// Memory combined writes.
    ///
    /// Buffer writes will be combined for possible larger bus transactions.
    /// It's not advised to use these heaps for reading back data.
    const WRITE_COMBINED = 0x10,

    const LAZILY_ALLOCATED = 0x20,
}

For Data we need DEVICE_LOCAL, we also want to avoid CPU_VISIBLE to keep it for other uses.
For Upload we need CPU_VISIBLE and we aim for WRITE_COMBINED without CPU_CACHED.
For Download we need CPU_VISIBLE and we aim for CPU_CACHED without WRITE_COMBINED.
Now Dynamic could leverage DEVICE_LOCAL & CPU_VISIBLE to upload without staging.

Once we are able to manage invalidations and flushes, COHERENT shouldn't really matter. But it will be a time saver for the first implementation.
Should we check that what we select is not LAZILY_ALLOCATED ?

Resource heap type

We also have to pick a resource type. We could use Any when available for more flexiblity and fall back to specific types otherwise, or we could always use the specific types. If I understand correctly Targets will be necessary for Images with the RENDER_TARGET bind.

pub enum ResourceHeapType {
    Any,
    Buffers,
    Images,
    Targets,
}

Allocation API

The most convenient for the user would be to keep the old API where you simply ask for a buffer or an image and memory allocation is just taken care of. But this requires a good general-purpose allocation strategy and might hurt performances. A compromise could be to expose different optional allocation strategies or hints when you want more control, or even allow you to write your own. However the latter would probably require a Box<Fn> callback on buffer and image drops, unless a channel returning information like some kind of offset would be suitable.

For example you could decide to bypass the default allocator and use a simple StackAllocator where memory is only acquired and released in a stack fashion if it fits your needs for a bunch of objects.

We could start with a simple default allocator that would be improved with time, and provide other allocators when good use-cases arise.

@msiglreith
Copy link
Contributor

I vote for keeping the old Usage enums, which way easier to understand compared to the heap properties exposed by vulkan. Probably even strong type these buffers/images according to their usage?

I'm unsure about exposing heaps, but for the moment we should do it and see how much overhead they introduce. HeapTypes should be always required, reducing feature support check (required for D3D12 TIER_1 devices).

@kvark
Copy link
Member

kvark commented Sep 11, 2017

Thanks for writing this down @Bastacyclop , it's great to have as a basis for discussion and moving forward!

For Dynamic, I vote to rename it to Hybrid, since the semantic is changed, and we don't want user to actually rely on it given that not a lot of platforms support shared CPU-GPU memory.

For the allocation API, please please make it simple, or at least - start with something dead simple. Even specifying Any for ResourceHeapType is fine to start with, assuming that will get us the old examples ported/working sooner, assuming we can adjust the implementation later to be more sophisticated and have different heaps for different resource types.

@Bastacyclop
Copy link
Contributor Author

Bastacyclop commented Sep 11, 2017

HeapTypes should be always required, reducing feature support check

I don't really understand what that means, aren't the heap types always exposed by core::Gpu ?

For Dynamic, I vote to rename it to Hybrid

Sounds good

start with something dead simple

I'll do my best ;)

@msiglreith
Copy link
Contributor

I don't really understand what that means, aren't the heap types always exposed by core::Gpu ?

Right, I meant ResourceHeapType 😄

@kvark
Copy link
Member

kvark commented Apr 25, 2019

Don't think this is relevant any more, with pre-ll being deprecated.

@kvark kvark closed this as completed Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants