Skip to content
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

Use strong references for JTF dependencies. #847

Merged
merged 1 commit into from May 11, 2021

Conversation

lifengl
Copy link
Member

@lifengl lifengl commented May 7, 2021

Consider in Dev 17.

Move away from WeakReferenceDictionary, and switch to strong reference in JTF dependency graph. This would reduce the complexity/overhead of the data structure & also makes it much easier to walk through the dependencies tree in a debug session.

Weak reference gives very little value to us here, because:
1, all incompleted JTF tasks are referenced by a global strong reference table inside JTF Context, so a weak reference to them will lead them to be GCed.
2, when JTF task is completed, it always removes itself from the graph

Basically, a weak reference to a JTF task inside JTF dependency graph never gives any memory benefit.

A weak reference to a JTF Collection does give some benefit, if it is no longer used and empty, but is still held by a not completed JTF task (Joins to a collection). In that case, the weak reference does allow the JTF collection to be GCed. But in the real product, this is fairly rare, and the memory usage of an empty JTF collection is limited, comparing the overhead of using weak reference dictionary in all JTF tasks/collections to track their dependencies.

(Thought about the change for a while, it sounds to me Dev 17 is a good time for it.)

Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

I don't have strong understanding of the dependency graph, but your argument sounds well-thought out. I trust you'll respond if this turns out to leak or consume too much memory.

@lifengl
Copy link
Member Author

lifengl commented May 11, 2021

Yes, i think getting into the early Dev 17 will give enough time to find out potential problems. JTF collections can be named, and JTF tasks hold init function for debug propose, so hopefully, it makes it easier to investigate.

BTW, in a few rare cases, holding the initial function leads to memory issues when it happens to hold a large closure, and it becomes a long running task. But it is relatively rare, and the benefit of using it to investigate dumps seems to exceed the risk so far.

@lifengl lifengl merged commit 1bbd7b2 into main May 11, 2021
@lifengl lifengl deleted the dev/lifengl/moveAwayWeakDependencies branch May 11, 2021 18:06
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.

None yet

2 participants