ss: fix QuarkIterator
Bug 531766
it would catch statesystemdisposedexceptions and throw
NoSuchElementExceptions instead.
Pre-fetching and storing the next and current intervals when
hasNext/Previous is called allows for safer handling of SSDEs
Change-Id: Ic3d79976f484d882972be9e17c03c82762ba7f08
Signed-off-by: Loic Prieur-Drevon <loic.prieurdrevon@gmail.com>
Reviewed-on: https://git.eclipse.org/r/118903
Tested-by: Hudson CI
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
Tested-by: Patrick Tasse <patrick.tasse@gmail.com>
diff --git a/analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/cpuusage/CpuUsageDataProvider.java b/analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/cpuusage/CpuUsageDataProvider.java
index 0d25bc2..8b29592 100644
--- a/analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/cpuusage/CpuUsageDataProvider.java
+++ b/analysis/org.eclipse.tracecompass.analysis.os.linux.core/src/org/eclipse/tracecompass/analysis/os/linux/core/cpuusage/CpuUsageDataProvider.java
@@ -260,7 +260,7 @@
/* Find a name in this attribute's intervals */
Iterator<ITmfStateInterval> iterator = new StateSystemUtils.QuarkIterator(kernelSs, execNameQuark, start);
Iterator<String> names = Iterators.filter(Iterators.transform(iterator, ITmfStateInterval::getValue), String.class);
- if (iterator.hasNext()) {
+ if (names.hasNext()) {
execName = names.next();
fProcessNameMap.put(tid, execName);
return execName;
diff --git a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/statesystem/core/StateSystemUtils.java b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/statesystem/core/StateSystemUtils.java
index 57c34cd..d6a5eaf 100644
--- a/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/statesystem/core/StateSystemUtils.java
+++ b/statesystem/org.eclipse.tracecompass.statesystem.core/src/org/eclipse/tracecompass/statesystem/core/StateSystemUtils.java
@@ -19,6 +19,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.NoSuchElementException;
+import java.util.Objects;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
@@ -297,7 +298,18 @@
private final long fInitialTime;
private final long fEndTime;
+ /**
+ * The last returned interval
+ */
private @Nullable ITmfStateInterval fCurrent;
+ /**
+ * The previous interval (pre-fetched)
+ */
+ private @Nullable ITmfStateInterval fPrevious;
+ /**
+ * The next interval (pre-fetched)
+ */
+ private @Nullable ITmfStateInterval fNext;
/**
* Constructor
@@ -369,6 +381,9 @@
@Override
public boolean hasNext() {
+ if (fNext != null) {
+ return true;
+ }
/*
* Compute the query's real end time here, to update it if the
* iterator is used during state system build
@@ -380,18 +395,26 @@
* query time range. By definition getNextQueryTime() is larger than
* the state system's start time.
*/
- return (getNextQueryTime() <= end);
+ long nextQueryTime = getNextQueryTime();
+ if (nextQueryTime <= end) {
+ try {
+ fNext = fSS.querySingleState(nextQueryTime, fQuark);
+ } catch (StateSystemDisposedException e) {
+ fNext = null;
+ return false;
+ }
+ }
+ return fNext != null;
}
@Override
public ITmfStateInterval next() {
if (hasNext()) {
- try {
- fCurrent = fSS.querySingleState(getNextQueryTime(), fQuark);
- return fCurrent;
- } catch (StateSystemDisposedException e) {
- /* GOTO throw NoSuchElementException. */
- }
+ ITmfStateInterval next = Objects.requireNonNull(fNext, "Inconsistent state, should be non null if hasNext returned true"); //$NON-NLS-1$
+ fPrevious = fCurrent;
+ fCurrent = next;
+ fNext = null;
+ return next;
}
throw new NoSuchElementException();
}
@@ -404,12 +427,24 @@
* @return true if the iteration has more previous elements
*/
public boolean hasPrevious() {
+ if (fPrevious != null) {
+ return true;
+ }
/*
* Ensure that the next query time falls within state system and
* query time range. By definition getPreviousQueryTime() is smaller
* than the state system's end time.
*/
- return (getPreviousQueryTime() >= fSS.getStartTime());
+ long previousQueryTime = getPreviousQueryTime();
+ if (previousQueryTime >= fSS.getStartTime()) {
+ try {
+ fPrevious = fSS.querySingleState(previousQueryTime, fQuark);
+ } catch (StateSystemDisposedException e) {
+ fPrevious = null;
+ return false;
+ }
+ }
+ return fPrevious != null;
}
/**
@@ -419,12 +454,11 @@
*/
public ITmfStateInterval previous() {
if (hasPrevious()) {
- try {
- fCurrent = fSS.querySingleState(getPreviousQueryTime(), fQuark);
- return fCurrent;
- } catch (StateSystemDisposedException e) {
- /* GOTO throw NoSuchElementException. */
- }
+ ITmfStateInterval prev = Objects.requireNonNull(fPrevious, "Inconsistent state, should be non null if hasPrevious returned true"); //$NON-NLS-1$
+ fNext = fCurrent;
+ fCurrent = prev;
+ fPrevious = null;
+ return prev;
}
throw new NoSuchElementException();
}
