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

Data Management RFC #1158

Closed
wants to merge 9 commits into from
Closed

Data Management RFC #1158

wants to merge 9 commits into from

Conversation

tsherif
Copy link
Contributor

@tsherif tsherif commented Jul 3, 2019

A system to manage data sets and create buffers or textures from them. Supports caching and re-using resources. Would ideally take over some of the complexity that currently lives in deck.gl's AttributeManager.

@coveralls
Copy link

coveralls commented Jul 3, 2019

Coverage Status

Coverage remained the same at 41.212% when pulling 13864ec on data-management-rfc into 0dabf7f on master.

## Implementation

A `DataManager` class that supports the following methods:
- `addData(id, data, accessors)`: add a data set to the manager, optionally with accessors to create particular views.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel that this is just moving the complexity from AttributeManager to DataManager. I'd prefer a minimal class that is just ResourceManager and not deal with data creation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that the caching requires identifying when the same data is being used. If the accessors are too much, I could just make it take a typed array or an image (i.e. all the parsing would be done in deck, and then the value arrays would be registered with the DataManager).

Copy link
Collaborator

Choose a reason for hiding this comment

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

In deck.gl we do update a typed array in-place before updating the buffer. It's a poor indicator for using cached result since the content of the same typed array and the same buffer may be out of sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... maybe it would be better to make the API closer to the ProgramManager then. Something like dm.addBuffer('myBuffer', buffer), where the application would be responsible for providing a key that uniquely identifies a buffer. And all the other operations would be based on buffer ids. Might be tricky to do caching for data, but things like Geometry attributes would be easy to cache.

dm.addData('anotherImage', image3); // Accessors not provided because the only view is the entire data set

const positionBuffer = dm.getBuffer('myTable', 'positions');
const positionBuffer2 = dm.getBuffer('myTable', 'positions'); // Gets cached version of same buffer
Copy link
Collaborator

Choose a reason for hiding this comment

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

In deck.gl Attribute class we need the following operations:

  • Allocate an empty buffer of a specific size
  • Create a new buffer from TypedArray
  • Resize an existing buffer (keep the data but allocate bigger size)
  • Partially update an existing buffer
  • Copy a chunk from one buffer to another

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These should all be doable.

@tsherif
Copy link
Contributor Author

tsherif commented Jul 3, 2019

@Pessimistress Got rid of accessors and added the methods you asked for.

@tsherif tsherif closed this Sep 26, 2019
@Pessimistress Pessimistress deleted the data-management-rfc branch November 16, 2020 23:49
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.

None yet

3 participants