callstack: Add synchronization on fields in data provider
Commit ef3c56e87c797f7a41e5d717fdc5b37266f95a60 removed the locking from
the data provider, but it was not entirely unnecessary. Some
synchronization is required on the fIdToCallstack field to ensure there
are no ConcurrentModificationException, but locking is introduced in
micro-dose only where necessary.
Change-Id: Ia6eb942e5907c4374b9a0e867738516e6618a679
Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
Reviewed-on: https://git.eclipse.org/r/156091
Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org>
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
diff --git a/callstack/org.eclipse.tracecompass.incubator.callstack.core/src/org/eclipse/tracecompass/incubator/internal/callstack/core/instrumented/provider/FlameChartDataProvider.java b/callstack/org.eclipse.tracecompass.incubator.callstack.core/src/org/eclipse/tracecompass/incubator/internal/callstack/core/instrumented/provider/FlameChartDataProvider.java
index 8702529..70e23b7 100644
--- a/callstack/org.eclipse.tracecompass.incubator.callstack.core/src/org/eclipse/tracecompass/incubator/internal/callstack/core/instrumented/provider/FlameChartDataProvider.java
+++ b/callstack/org.eclipse.tracecompass.incubator.callstack.core/src/org/eclipse/tracecompass/incubator/internal/callstack/core/instrumented/provider/FlameChartDataProvider.java
@@ -242,12 +242,25 @@
@Override
public TmfModelResponse<List<ITimeGraphArrow>> fetchArrows(Map<String, Object> fetchParameters, @Nullable IProgressMonitor monitor) {
List<ITmfStateInterval> arrows = fArrowProvider.fetchArrows(fetchParameters, monitor);
+ if (monitor != null && monitor.isCanceled()) {
+ return new TmfModelResponse<>(null, Status.CANCELLED, CommonStatusMessage.TASK_CANCELLED);
+ }
+ if (arrows.isEmpty()) {
+ return new TmfModelResponse<>(Collections.emptyList(), Status.COMPLETED, CommonStatusMessage.COMPLETED);
+ }
List<ITimeGraphArrow> tgArrows = new ArrayList<>();
// First, get the distinct callstacks
- Set<CallStack> callstacks = fIdToCallstack.values().stream()
- .map(CallStackDepth::getCallStack)
- .distinct()
- .collect(Collectors.toSet());
+ List<CallStackDepth> csList = new ArrayList<>();
+ synchronized (fIdToCallstack) {
+ // Quick copy the values to a list to avoid keeping the lock for too
+ // long, adding to a hashSet takes time to calculate the CallStack's
+ // hash.
+ csList.addAll(fIdToCallstack.values());
+ }
+ Set<CallStack> callstacks = new HashSet<>();
+ for (CallStackDepth csd : csList) {
+ callstacks.add(csd.getCallStack());
+ }
// Find the source and destination entry for each arrow
for (ITmfStateInterval interval : arrows) {
@@ -283,12 +296,9 @@
}
// We found the callstack, find the right depth and its entry id
int currentDepth = callstack.getCurrentDepth(ts);
- for (Entry<Long, CallStackDepth> csdEntry : fIdToCallstack.entrySet()) {
- CallStackDepth csd = csdEntry.getValue();
- // For callstack, compare objects, not content
- if (csd.getDepth() == currentDepth && csd.getCallStack() == callstack) {
- return csdEntry.getKey();
- }
+ CallStackDepth csd = new CallStackDepth(callstack, currentDepth);
+ synchronized (fIdToCallstack) {
+ return fIdToCallstack.inverse().get(csd);
}
}
return null;
@@ -377,7 +387,9 @@
// Get an entry for a quark
private long getEntryId(CallStackDepth stack) {
- return fIdToCallstack.inverse().computeIfAbsent(stack, q -> ENTRY_ID.getAndIncrement());
+ synchronized (fIdToCallstack) {
+ return fIdToCallstack.inverse().computeIfAbsent(stack, q -> ENTRY_ID.getAndIncrement());
+ }
}
private long getEntryId(ICallStackElement instrumentedCallStackElement) {
