Skip to content

Commit 06fcd6f

Browse files
committed
Detect "shadowed" cache hits
1 parent e8bf7e3 commit 06fcd6f

File tree

1 file changed

+90
-0
lines changed

1 file changed

+90
-0
lines changed

extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,75 @@ class ComparisonDataset {
9797
pipelines: data.pipelineSummaryList[index],
9898
};
9999
}
100+
101+
/**
102+
* Returns the RA hashes of all predicates that were evaluated in this data set, but not seen in `other`,
103+
* because in `other` the dependency upon these predicates was cut off by a cache hit.
104+
*
105+
* For example, suppose predicate `A` depends on `B`, which depends on `C`, and the
106+
* predicates were evaluated in the first log but not the second:
107+
* ```
108+
* first eval. log second eval. log
109+
* predicate A seen evaluation seen cache hit
110+
* |
111+
* V
112+
* predicate B seen evaluation not seen
113+
* |
114+
* V
115+
* predicate C seen evaluation not seen
116+
* ```
117+
*
118+
* To ensure a meaningful comparison, we want to omit `predicate A` from the comparison view because of the cache hit.
119+
*
120+
* But predicates B and C did not have a recorded cache hit in the second log, because they were never scheduled for evaluation.
121+
* Given the dependency graph, the most likely explanation is that they would have been evaluated if `A` had not been a cache hit.
122+
* We therefore say that B and C are "shadowed" by the cache hit on A.
123+
*
124+
* The dependency graph is only visible in the first evaluation log, because `B` and `C` do not exist in the second log.
125+
* So to compute this, we use the dependency graph from one log together with the set of cache hits in the other log.
126+
*/
127+
getPredicatesShadowedByCacheHit(other: ComparisonDataset) {
128+
const {
129+
data: { dependencyLists, raHashes, names },
130+
raToIndex,
131+
} = this;
132+
const cacheHits = new Set<string>();
133+
134+
function visit(index: number, raHash: string) {
135+
if (cacheHits.has(raHash)) {
136+
return;
137+
}
138+
cacheHits.add(raHash);
139+
const dependencies = dependencyLists[index];
140+
for (const dep of dependencies) {
141+
const name = names[dep];
142+
if (!other.nameToIndex.has(name)) {
143+
visit(dep, raHashes[dep]);
144+
}
145+
}
146+
}
147+
148+
for (const otherCacheHit of other.cacheHitIndices) {
149+
{
150+
// Look up by RA hash
151+
const raHash = other.data.raHashes[otherCacheHit];
152+
const ownIndex = raToIndex.get(raHash);
153+
if (ownIndex != null) {
154+
visit(ownIndex, raHash);
155+
}
156+
}
157+
{
158+
// Look up by name
159+
const name = other.data.names[otherCacheHit];
160+
const ownIndex = this.nameToIndex.get(name);
161+
if (ownIndex != null) {
162+
visit(ownIndex, this.data.raHashes[ownIndex]);
163+
}
164+
}
165+
}
166+
167+
return cacheHits;
168+
}
100169
}
101170

102171
function renderOptionalValue(x: Optional<number>, unit: string | undefined) {
@@ -503,6 +572,17 @@ function ComparePerformanceWithData(props: {
503572

504573
const hasCacheHitMismatch = useRef(false);
505574

575+
const shadowedCacheHitsFrom = useMemo(
576+
() =>
577+
hideCacheHits ? from.getPredicatesShadowedByCacheHit(to) : new Set(),
578+
[from, to, hideCacheHits],
579+
);
580+
const shadowedCacheHitsTo = useMemo(
581+
() =>
582+
hideCacheHits ? to.getPredicatesShadowedByCacheHit(from) : new Set(),
583+
[from, to, hideCacheHits],
584+
);
585+
506586
const rows: Row[] = useMemo(() => {
507587
hasCacheHitMismatch.current = false;
508588
return Array.from(keySet)
@@ -523,6 +603,16 @@ function ComparePerformanceWithData(props: {
523603
return undefined!;
524604
}
525605
}
606+
if (
607+
(isPresent(before) &&
608+
!isPresent(after) &&
609+
shadowedCacheHitsFrom.has(before.raHash)) ||
610+
(isPresent(after) &&
611+
!isPresent(before) &&
612+
shadowedCacheHitsTo.has(after.raHash))
613+
) {
614+
return undefined!;
615+
}
526616
const diff =
527617
(isPresent(afterValue) ? afterValue : 0) -
528618
(isPresent(beforeValue) ? beforeValue : 0);

0 commit comments

Comments
 (0)