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

use a shared object with multiple length keys to store regl buffers #152

Closed
wants to merge 1 commit into from

Conversation

bmschmidt
Copy link
Collaborator

@bmschmidt bmschmidt commented Oct 1, 2024

Previously deepscatter had something called a TileBufferManager. This doesn't make a huge amount of sense, and had a concrete cost; if two tiles used the same underlying Arrow memory for a column (say, 2**16 floating point zeros), then we'd use different memory on the GPU for them.

This PR revamps that storage so that it uses the new TupleMap class. There is a single shared buffer manager across the whole plot, which uses a key that includes the tile itself and then the string name of the field desired (which is now actually string[], for help in the next PR.)

This map associates each column with its underlying Arrow vector memory, and it is that memory that is used as a key to the GPU buffers. This is a little weird, but the two-step operation means taht we can re-use memory more efficiently.

At some later point I'll more fully build on this. One goal is that for four types of floating point data we can always use a single webGL buffer instead of re-allocating them ad-infinitum:

All-false (or all-zero in float32 land)
All-true (or all-one in float32 land)
Nth-item-true, rest false (by creating a big array with a one in the middle at position K, and starting the view of that buffer at position K-N.
Nth-item-false, rest true (inverse of number 3).


Important

Introduces BufferManager for efficient GPU buffer management in regl_rendering.ts, replacing TileBufferManager, and updates rendering logic in ReglRenderer.

  • Buffer Management:
    • Introduced BufferManager in regl_rendering.ts to replace TileBufferManager for managing GPU buffers.
    • Uses TupleMap to associate Arrow vector memory with GPU buffers, allowing shared buffer management across the plot.
    • BufferManager handles buffer creation and memory reuse, improving efficiency.
  • Rendering Changes:
    • Updated ReglRenderer to use BufferManager for buffer management.
    • Adjusted buffer handling logic in render_points() and remake_renderer().
  • Miscellaneous:
    • Removed 'observable10' from color schemes in ColorChange.svelte.

This description was created by Ellipsis for 5903dbd. It will automatically update as commits are pushed.

Copy link
Collaborator Author

bmschmidt commented Oct 1, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bmschmidt and the rest of your teammates on Graphite Graphite

This was referenced Oct 1, 2024
@bmschmidt bmschmidt marked this pull request as ready for review October 1, 2024 01:03
@bmschmidt bmschmidt closed this Oct 1, 2024
@@ -45,6 +41,8 @@ import { Color } from './aesthetics/ColorAesthetic';
import { StatefulAesthetic } from './aesthetics/StatefulAesthetic';
import { Filter, Foreground } from './aesthetics/BooleanAesthetic';
import { ZoomTransform } from 'd3-zoom';
import { TupleMap } from './utilityFunctions';
import { buffer } from 'd3';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import 'buffer' from 'd3' is unused and can be removed to clean up the code.

@@ -123,7 +120,8 @@ export class Tile {

deleteColumn(colname: string) {
if (this._batch) {
this._buffer_manager?.release(colname);
console.warn('Deleting column from tile doesnt free GPU memory');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting a column from a tile does not free GPU memory. Consider implementing a mechanism to release GPU resources when a column is deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant