Decref the members of Runtime(split from #958) #1019
Conversation
* Unified GetBuiltins method
Codecov Report
@@ Coverage Diff @@
## master #1019 +/- ##
=======================================
Coverage 86.71% 86.71%
=======================================
Files 1 1
Lines 301 301
=======================================
Hits 261 261
Misses 40 40
Continue to review full report at Codecov.
|
| @@ -393,6 +415,19 @@ internal static int AtExit() | |||
| return 0; | |||
| } | |||
|
|
|||
| private static void SetPyMember(ref IntPtr obj, IntPtr value, Action onRelease) | |||
| /// </summary> | ||
| public void Add(IntPtr ob, Action onRelease) | ||
| { | ||
| _actions.Add(new KeyValuePair<IntPtr, Action>(ob, onRelease)); |
There was a problem hiding this comment.
NIT: use Tuple<T1, T2>, e.g. just _actions.Add((ob, onRelease))
There was a problem hiding this comment.
Why using Tuple? As my experience, with two elements cases, KeyValue would always faster than tuple(unless the struct is too big).
There was a problem hiding this comment.
Simply for readability. This code is not a hot path, so a little performance difference hardly matters.
Also, once we retarget to newer framework, C# will use ValueTuple anyway.
| foreach (var item in _actions) | ||
| { | ||
| Runtime.XDecref(item.Key); | ||
| item.Value?.Invoke(); | ||
| } |
There was a problem hiding this comment.
NIT: use foreach(var (ob, onRelease) in _actions) (requires modification from previous comment).
What does this implement/fix? Explain your changes.
Does this close any currently open issues?
...
Any other comments?
...
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG