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

Simplify code in RapidTypeAnalysisAlgorithm #1167

Merged
merged 3 commits into from
Feb 11, 2025
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* #L%
*/

import com.google.common.collect.ArrayListMultimap;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -49,7 +50,7 @@
public class RapidTypeAnalysisAlgorithm extends AbstractCallGraphAlgorithm {

@Nonnull protected Set<ClassType> instantiatedClasses = Collections.emptySet();
@Nonnull protected Map<ClassType, List<Call>> ignoredCalls = Collections.emptyMap();
@Nonnull protected ArrayListMultimap<ClassType, Call> ignoredCalls = ArrayListMultimap.create();
Comment on lines 52 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we make them final and initialize them just in the constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have the same thought.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it isnt final since it is a big data structure which is only needed in the creation. The Map is thrown away after the processing.
I guess if we want to make it final we have to clear the Map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can replace ignoredCalls = ... with ignoredCalls.clear() and make it final.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I solved the issue of RTA only using initialized Classes and the On the fly creation by creating a ignored calls Map, that saves possible edges for a specific class, if the algorithm ever sees an initilaiziation of the class all possible edges are taken from the Map and the entry is cleared.
So we can still process every method only once and still adapt the edges to later found information.
But This approach is quite memory heavy, so I clear the Map after the algorithm is done.

Copy link
Collaborator

@swissiety swissiety Jan 24, 2025

Choose a reason for hiding this comment

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

is it necessary to have that temporary map as a field or does make sense if we have some kind of (temporary) builder class that stores that information etc.? we could move CallGraph.addCall there as well


/**
* The constructor of the RTA algorithm.
Expand All @@ -72,13 +73,13 @@ public CallGraph initialize() {
public CallGraph initialize(@Nonnull List<MethodSignature> entryPoints) {
// init helper data structures
instantiatedClasses = new HashSet<>();
ignoredCalls = new HashMap<>();
ignoredCalls = ArrayListMultimap.create();

CallGraph cg = constructCompleteCallGraph(entryPoints);

// delete the data structures
instantiatedClasses = Collections.emptySet();
ignoredCalls = Collections.emptyMap();
ignoredCalls = ArrayListMultimap.create();
return cg;
}

Expand Down Expand Up @@ -198,13 +199,8 @@ private Stream<MethodSignature> resolveAllCallTargets(
private void saveIgnoredCall(
MethodSignature source, MethodSignature target, InvokableStmt invokableStmt) {
ClassType notInstantiatedClass = target.getDeclClassType();
List<Call> calls = ignoredCalls.get(notInstantiatedClass);
Call ignoredCall = new Call(source, target, invokableStmt);
if (calls == null) {
calls = new ArrayList<>();
ignoredCalls.put(notInstantiatedClass, calls);
}
calls.add(ignoredCall);
ignoredCalls.put(notInstantiatedClass, ignoredCall);
}

/**
Expand Down Expand Up @@ -247,24 +243,22 @@ protected void preProcessingMethod(
protected void includeIgnoredCallsToClass(
ClassType classType, MutableCallGraph cg, Deque<MethodSignature> workList) {
List<Call> newEdges = ignoredCalls.get(classType);
if (newEdges != null) {
newEdges.forEach(
call -> {
MethodSignature concreteTarget =
resolveConcreteDispatch(view, call.getTargetMethodSignature()).orElse(null);
if (concreteTarget == null) {
return;
}
addCallToCG(
call.getSourceMethodSignature(),
concreteTarget,
call.getInvokableStmt(),
cg,
workList);
});
// can be removed because the instantiated class will be considered in future resolves
ignoredCalls.remove(classType);
}
newEdges.forEach(
call -> {
MethodSignature concreteTarget =
resolveConcreteDispatch(view, call.getTargetMethodSignature()).orElse(null);
if (concreteTarget == null) {
return;
}
addCallToCG(
call.getSourceMethodSignature(),
concreteTarget,
call.getInvokableStmt(),
cg,
workList);
});
// can be removed because the instantiated class will be considered in future resolves
ignoredCalls.removeAll(classType);
}

/**
Expand Down