Skip to content

Narrowing Blob#11167

Closed
smessmer wants to merge 7 commits intomasterfrom
export-D9623916
Closed

Narrowing Blob#11167
smessmer wants to merge 7 commits intomasterfrom
export-D9623916

Conversation

@smessmer
Copy link
Contributor

@smessmer smessmer commented Sep 1, 2018

Stack:
    :black_circle:  #11167 Narrowing Blob  💛
    :white_circle:  #11238 Some improvements to IValue  💛
    :white_circle:  #11258 Improve Tensor() constructor  💛
    :white_circle:  #11260 Fix intrusive_ptr move/copy for different NullType's  💛
    :white_circle:  #11294 Remove manual refcounting from Tensor class  💛
    :white_circle:  #11352 Remove intrusive_ptr::reclaim() in Storage  💛
    :white_circle:  #11353 Simplify union payload copying  💛
    :white_circle:  #11355 Simplify IValue::toTensor()  💛
    :white_circle:  #11402 Simplify Blob move constructor/assignment  💛
    :white_circle:  #11414 IValue can store Blob  💛
    :white_circle:  #11481 Blob doesn't allow access to destroyCall anymore  💛

Narrow the Blob API as preparation for merging Blob/IValue

  • get rid of templated IsType and Operator::InputIsType / OutputIsType
  • Use 'using' instead of 'typedef' for DestroyCall (just for readability)

Differential Revision: D9623916

Differential Revision: D9623916
Differential Version: 56705898
@zou3519 zou3519 added the caffe2 label Sep 1, 2018
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Don't forget to say in your commit message what you actually narrowed. (In this case, you got rid of templated IsType and changed the deleter type... be more specific!)

TypeMeta meta_;
void* pointer_ = nullptr;
DestroyCall destroy_ = nullptr;
DestroyCall* destroy_ = nullptr;

This comment was marked as off-topic.

This comment was marked as off-topic.

@ezyang
Copy link
Contributor

ezyang commented Sep 2, 2018

Also, if a codemod was used, put it in the commit description too.

@smessmer
Copy link
Contributor Author

smessmer commented Sep 4, 2018

I expanded the description. Didn't use a codemod but changed the call sites manually.

Differential Revision: D9623916
Differential Version: 56858422
Differential Revision: D9623916
Differential Version: 56900314
Differential Revision: D9623916
Differential Version: 56982469
Differential Revision: D9623916
Differential Version: 57101548
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

smessmer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Differential Revision: D9623916
Differential Version: 57135223
Differential Revision: D9623916
Differential Version: 57214033
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
Summary:
Pull Request resolved: pytorch#11167

Narrow the Blob API as preparation for merging Blob/IValue

- get rid of templated IsType and Operator::InputIsType / OutputIsType
- Use 'using' instead of 'typedef' for DestroyCall (just for readability)

Reviewed By: ezyang

Differential Revision: D9623916

fbshipit-source-id: 952f0b0cf5a525094b02e8d2798dd57a56a9e1d8
@ezyang ezyang added the merged label Jun 26, 2019
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.

4 participants