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

Rework Canvas to Reduce Allocations #568

Closed
ilexp opened this Issue Oct 1, 2017 · 2 comments

Comments

1 participant
@ilexp
Member

ilexp commented Oct 1, 2017

Summary

As became apparent while working on issue #566, the Canvas class could be redesigned with an API and internal structure that allows to avoid allocations. Prototype this and see how well it works.

Analysis

  • Allow switching device targets, so Canvas users can use the same instance continuously.
  • Integrate CanvasBuffer into Canvas directly.
  • Buffer the state stack, so repeated push / pop calls won't cause allocations.
  • Update all the spots in code where Canvas is used to re-use it every frame.
@ilexp

This comment has been minimized.

Member

ilexp commented Oct 8, 2017

Progress

  • Optimized Canvas PushState / PopState behavior to internally re-use the same instances when growing and shrinking the stack repeatedly.
  • Moved to a Begin(device) / End usage model for Canvas, favoring re-use of the same instance across frames and devices.
  • Updated all spots in the codebase that were affected by this.
  • Integrated CanvasBuffer directly into Canvas and changed buffering behavior to use only a single shared vertex buffer that is re-used for every AddVertices call - which in now possible, because IDrawDevice is guaranteed to make a copy.
  • Benchmark results:

Immediate ToDo

  • The biggest memory sink right now is instantiating temporary BatchInfo instances and initializing their internal value array.
    • They are re-instantiated on every use, even though they could probably be shared and re-used across frames as long as two BatchInfo instances are equal.
  • Would it make sense to maintain a "temporary material store" somewhere that maps a (long) material hash to a shared instance?
    • In fact, it would theoretically be possible to have a central, runtime-constant mapping from hashes to read-only materials. This wouldn't be a good design, but it hints at a potential approach for optimization and avoiding creating so many instances.
    • It would be possible to have only one temporary, writable BatchInfo instance, and a shared, readonly BatchInfo cache with a hash-to-instance mapping. The question is where to put that mapping and how to design an API around this.
    • Edit: Then again, this would break critically when it comes to materials which have per-frame adjusted float values that likely never reach exactly the same state, producing an infinite number of different BatchInfo instances in a mapping like this.
  • Would it make sense to allow renderers to rent temporary materials from an IDrawDevice instance?
    • The drawing device could easily pool them.
    • No need to explicitly release them, as the drawing device knows when the rendering has been finished, i.e. when the old temp materials are no longer used.
    • Side note: IDrawDevice is really getting quite big. See if it can be reduced.
  • Other options and optimization approaches?

@ilexp ilexp self-assigned this Oct 8, 2017

@ilexp

This comment has been minimized.

Member

ilexp commented Oct 11, 2017

Progress

  • Introduced temporary BatchInfo API (RentMaterial) to IDrawDevice and provided some related extension methods for added convenience. It is not mandatory - people can still just create their instances every frame if they want, but now there's a(n amortized) zero-alloc way for this too.
  • Implemented material renting as a pool in DrawDevice which gets reset at the end of the Render call.
  • Used temporary / rented materials in Canvas and everywhere else in the codebase where it made sense.
  • Benchmark results:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment