Skip to content

Fix local objects detector #422

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

Merged
merged 7 commits into from
Nov 25, 2024
Merged

Fix local objects detector #422

merged 7 commits into from
Nov 25, 2024

Conversation

eupp
Copy link
Collaborator

@eupp eupp commented Nov 7, 2024

This PR changes our approach to the local objects tracking.

Current approach maintains a map LocalObject -> List<LocalObject> , which for each local object stores a list of objects immediately reachable from it (via its fields). This map is used to compute a set of local objects reachable from another given local object. Once a local object is assigned to a field of non-local object, all reachable objects are marked as non-local.

The problem with this approach is that it is error prone and requires a lot of efforts to maintain the map in the correct state. If the map is not maintained in the correct state, some objects might be incorrectly classified as local (see new test added in this PR).

So instead, in this PR we implement an alternative approach. We still maintain a set of local objects, however when we need to compute a set of objects reachable from given one, we instead traverse the object graph using the utility function.
Thus there is no need to maintain a separate data structure approximating reachable objects.

This PR also should help with #415 , as it would be much more easier to just traverse all objects reachable from the given Thread objects once it starts, and mark them as non-local.

Note this PR depends on PR #420 , so we first need to merge it, and then rebase.

@eupp eupp requested a review from ndkoval November 7, 2024 18:07
f.isEnumConstant ||
f.name == "serialVersionUID"
) return@traverseObjectGraph null
if (f.isEnumConstant || f.name == "serialVersionUID")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please always use brackets when the return statement is not on the same line

Copy link
Collaborator

@ndkoval ndkoval left a comment

Choose a reason for hiding this comment

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

The PR looks good, please address the only minor comment and rebase when #420 is merged

Base automatically changed from object-graph-traversing-fix to develop November 25, 2024 14:26
@eupp eupp force-pushed the fix-local-objects-detector branch from 77cc10b to 55a6833 Compare November 25, 2024 15:49
@eupp eupp requested a review from ndkoval November 25, 2024 17:29
@ndkoval ndkoval merged commit 43f3c43 into develop Nov 25, 2024
15 checks passed
@ndkoval ndkoval deleted the fix-local-objects-detector branch November 25, 2024 18:02
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.

2 participants