Skip to content

Conversation

@axinging
Copy link
Contributor

@axinging axinging commented Mar 8, 2021

FEATURE


This change is Reviewable

@axinging
Copy link
Contributor Author

axinging commented Mar 8, 2021

@gyagp @qjia7 @haoyunfeix @xhcao PTAL.

toDispose.push(sliced);
}

toDispose.forEach(t => backend.disposeData(t));
Copy link
Contributor

Choose a reason for hiding this comment

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

backend.disposeData(t.dataId)

Copy link
Contributor

Choose a reason for hiding this comment

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

I just found in the Pack implementation. You also used backend.disposeData(t) not backend.disposeData(t.dataId). Please change that part too. Otherwise, it may result memory leak.

Copy link
Contributor Author

@axinging axinging Mar 9, 2021

Choose a reason for hiding this comment

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

Great catch! Thanks Jiajia, updated, PTAL

Copy link
Contributor

@qjia7 qjia7 left a comment

Choose a reason for hiding this comment

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

LGTM.
@lina128 takes another look. Thanks.

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @qjia7)

@qjia7 qjia7 merged commit 85c9ae0 into tensorflow:master Mar 9, 2021
@axinging axinging deleted the unpack_webgpu branch May 25, 2021 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants