Fix reference count if array used in JIT operations.#3167
Merged
9prady9 merged 6 commits intoarrayfire:masterfrom Aug 18, 2021
Merged
Fix reference count if array used in JIT operations.#31679prady9 merged 6 commits intoarrayfire:masterfrom
9prady9 merged 6 commits intoarrayfire:masterfrom
Conversation
umar456
force-pushed
the
ref_count
branch
7 times, most recently
from
f872e20 to
6312353
Compare
umar456
changed the title
Member
|
Also I forgot to mention this earlier. median_cpu and canny_cpu are failing consistently across all CPU jobs on Ubuntu 20.04 now. |
9prady9
requested changes
Aug 13, 2021
Member
9prady9
left a comment
9prady9
left a comment
There was a problem hiding this comment.
Couple of more minor improvements.
umar456
force-pushed
the
ref_count
branch
5 times, most recently
from
5e4d034 to
319c411
Compare
The Node_map_t unordered_map object uses the pointer of the nodes for the key. This worked because you could previously because the node buffer objects tracked the buffer object's shared pointer. This required holding an additional reference to the buffer object when an Array was used in a JIT operation. This did not leak memory because both the buffer and the node were deleted when the Array object was destroyed. This commit creates a new hash function for the node pointers which dereferences the Node pointers and if they are buffers, it checks the buffer's pointer and its offset to determine if its unique. This approach allows us to remove the call_once construct from the setData member function of the buffer node. You can now create node objects for each invocation getNode function.
Previously when an af::array was used in a jit operation and it was backed by a buffer, a buffer node was created and the internal shared_ptr was stored in the Array for future use and returned when getNode was called. This increased the reference count of the internal buffer. This reference count never decreased because of the internal reference to the shared_ptr. This commit changes this behavior by createing new buffer nodes for each call the getNode. We use the new hash function to ensure the equality of the buffer node when the jit code is generated. This avoids holding the call_once flag in the buffer object and simplifies the management of the buffer node objects. Additionally when a jit node goes out of scope the reference count decrements as expected.
9prady9
approved these changes
Aug 18, 2021
Member
9prady9
left a comment
9prady9
left a comment
There was a problem hiding this comment.
Looks good but I have some queries about the node hashing, but we can discuss those over chat
jacobkahn
added a commit
to jacobkahn/flashlight
that referenced
this pull request
Feb 28, 2023
…hlight#1080) Summary: See title. These changes are due to arrayfire/arrayfire#3167, which updates and improves refcounting for ArrayFire arrays. Pull Request resolved: flashlight#1080 Test Plan: CI Reviewed By: richjames0 Differential Revision: D43633315 Pulled By: jacobkahn fbshipit-source-id: 8f838a42c4b724953900b8ccccde6dac23e137ec
facebook-github-bot
pushed a commit
to flashlight/flashlight
that referenced
this pull request
Mar 1, 2023
Summary: See title. These changes are due to arrayfire/arrayfire#3167, which updates and improves refcounting for ArrayFire arrays. Pull Request resolved: #1080 Test Plan: CI Reviewed By: richjames0 Differential Revision: D43633315 Pulled By: jacobkahn fbshipit-source-id: d8ed1592b1d57f3d357d40386a1c571110ad1950
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Better manage the reference count of an Array object when it is used in a JIT operation
Description
Previously when an af::array was used in a jit operation and it was backed by a
buffer, a buffer node was created and the internal shared_ptr was stored in the
Array for future use and returned when getNode was called. This increased the
reference count of the internal buffer. This reference count never decreased
because of the internal reference to the shared_ptr.
This commit changes this behavior by storing a weak_ptr instead of a
shared_ptr inernally to the Array object. This change reduces the
reference count once the Array is evaluated in an expression. If
the weak_ptr can be converted into a
Changes to Users
Enjoy lower reference counts. This may increase performance under certain scenarios
Checklist
[ ] Functions added to unified API