-
Notifications
You must be signed in to change notification settings - Fork 548
Fix reference count if array used in JIT operations. #3167
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
Conversation
f872e20
to
6312353
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I forgot to mention this earlier. median_cpu and canny_cpu are failing consistently across all CPU jobs on Ubuntu 20.04 now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of more minor improvements.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but I have some queries about the node hashing, but we can discuss those over chat
…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
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
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