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

Prep Attribute class for partial update #3091

Merged
merged 2 commits into from May 15, 2019
Merged

Conversation

Pessimistress
Copy link
Collaborator

For #1576

Change List

  • Avoid calculating buffer props unless needed
  • Only upload changed subarray on attribute update

@@ -170,6 +170,11 @@ export default class Attribute extends BaseAttribute {

this.constant = false;
this.value = new ArrayType(this.size * allocCount);

if (this.buffer && this.buffer.byteLength < this.value.byteLength) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ibgreen @tsherif This is the part I'm hoping to get some ideas - in the incremental update use case (e.g. data loaded by chunks), every time we reallocate the buffer, we need to re-upload the entire value array.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to know ahead of time in that incremental case what the total size of the data will be?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Generally, no.

Alternatively, every time overflow happens, we could over-allocate the buffer instead of setting it to the exact new size.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense to me. Something like doubling the size of the buffer on each overflow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How sensitive should we be regarding GPU memory usage?

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be a problem on mobile, but I think we have some leeway because we don't use textures much. It would be helpful to profile some typical use cases and see what our current memory usage is like.

@Pessimistress Pessimistress changed the title [For Discussion] Prep Attribute class for partial update Prep Attribute class for partial update May 15, 2019
@Pessimistress Pessimistress merged commit 9ffe6df into master May 15, 2019
@Pessimistress Pessimistress deleted the x/attribute-refactor branch May 15, 2019 20:47
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

2 participants