| commit | c723c30e5135ffaa1f3ac29967e6dd93ffb77e8b | |
| author | Shaunak Kishore <kshaunak@fb.com> | |
| Fri, 3 Apr 2020 05:19:21 +0000 (2 22:19 -0700) | ||
| committer | Facebook GitHub Bot <facebook-github-bot@users.noreply.github.com> | |
| Fri, 3 Apr 2020 05:41:49 +0000 (2 22:41 -0700) | ||
| tree | c5fef4b33abc88bbd60872cfd31394cf8969e79e | treesnapshot (tar.gz zip) |
| parent | c2cb633837da93edfa2976dc689c80c2a490184d | commitdiff |
GetPosKey should not inc-ref
Summary:
For consistency with GetPosVal, and just in general as a value-returning function, GetPosKey should not inc-ref the result.
I audited all callers of MixedArray::getKey, nvGetKey, and nvFirst to see if they were using the extra refcount and changed those that were to do a Variant::attach or similar. Most callers weren't making use of the extra refcount - for these callers, this change will fix a small leak to the GC.
There's only one place here where perf might matter: for the ArrayIter helpers, doing the inc-ref in MixedArrayElm might be faster... However, I think if GCC's analysis is good enough, we should get the same code (because we test the DataType and we set it to constants in the three branches - Int, String, and PersistentString). If this place ends up mattering I'll just add a getKeyIncRef to MixedArrayElm.
This diff is a lot smaller than the previous one, but also more delicate, hence splitting it out.
Reviewed By: ricklavoie
Differential Revision: D20820640
fbshipit-source-id: ae0055df8c972525ea9498d5241bad9cea41166c
Summary:
For consistency with GetPosVal, and just in general as a value-returning function, GetPosKey should not inc-ref the result.
I audited all callers of MixedArray::getKey, nvGetKey, and nvFirst to see if they were using the extra refcount and changed those that were to do a Variant::attach or similar. Most callers weren't making use of the extra refcount - for these callers, this change will fix a small leak to the GC.
There's only one place here where perf might matter: for the ArrayIter helpers, doing the inc-ref in MixedArrayElm might be faster... However, I think if GCC's analysis is good enough, we should get the same code (because we test the DataType and we set it to constants in the three branches - Int, String, and PersistentString). If this place ends up mattering I'll just add a getKeyIncRef to MixedArrayElm.
This diff is a lot smaller than the previous one, but also more delicate, hence splitting it out.
Reviewed By: ricklavoie
Differential Revision: D20820640
fbshipit-source-id: ae0055df8c972525ea9498d5241bad9cea41166c