Skip to content

Fix allocator bug#16712

Merged
sneaxiy merged 6 commits intoPaddlePaddle:developfrom
sneaxiy:fix_allocator_bug2
May 23, 2019
Merged

Fix allocator bug#16712
sneaxiy merged 6 commits intoPaddlePaddle:developfrom
sneaxiy:fix_allocator_bug2

Conversation

@sneaxiy
Copy link
Collaborator

@sneaxiy sneaxiy commented Apr 8, 2019

The previous architecture of Allocator would cause some Allocation cannot be released, saying potential memory leak. This PR fixes it.

For more details of Allocator design, please refer to paddle/fluid/memory/allocation/allocator.h in this PR.

CE has passed. Please see here.

@sneaxiy sneaxiy force-pushed the fix_allocator_bug2 branch from f5a9ae4 to 25a7221 Compare May 9, 2019 13:07
@sneaxiy sneaxiy force-pushed the fix_allocator_bug2 branch from cdbec37 to 5dd5c43 Compare May 20, 2019 05:59
@sneaxiy sneaxiy force-pushed the fix_allocator_bug2 branch from 5dd5c43 to d53b700 Compare May 21, 2019 01:50
@sneaxiy sneaxiy changed the title [Test] Fix allocator bug Fix allocator bug May 21, 2019
Allocation(const Allocation& o) = delete;
Allocation& operator=(const Allocation& o) = delete;
Allocation(Allocation&& o) = delete;
Allocation& operator=(Allocation&& o) = delete;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove those methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because Allocation should not be moved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because Allocation should not be moved. So I remove those methods generated by compiler.

Copy link
Contributor

@chengduoZH chengduoZH left a comment

Choose a reason for hiding this comment

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

Please add doc to explain the reason for adding InlinedVector in this PR.

Copy link
Contributor

@chengduoZH chengduoZH left a comment

Choose a reason for hiding this comment

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

LGTM

@sneaxiy
Copy link
Collaborator Author

sneaxiy commented May 23, 2019

@chengduoZH I would add some doc of InlinedVector in the next PR asap.

@sneaxiy sneaxiy merged commit c618963 into PaddlePaddle:develop May 23, 2019
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.

2 participants