From 090f79fc8480d8d2ffe1376559b12dcf1bee4f67 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 13 Nov 2024 13:39:04 +0100 Subject: [PATCH 1/7] Change to .tsx extension in RAPrettyPrinter --- .../{RAPrettyPrinter.ts => RAPrettyPrinter.tsx} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename extensions/ql-vscode/src/view/compare-performance/{RAPrettyPrinter.ts => RAPrettyPrinter.tsx} (100%) diff --git a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.ts b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx similarity index 100% rename from extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.ts rename to extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx From 89c113c7f33114e295de39b81ef89804847f740c Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 13 Nov 2024 13:58:20 +0100 Subject: [PATCH 2/7] Make RAPrettyPrinter generate JSX fragments --- .../ComparePerformance.tsx | 2 +- .../compare-performance/RAPrettyPrinter.tsx | 59 ++++++++++++++----- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx b/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx index 51555600d4a..c3664ff11af 100644 --- a/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx @@ -190,7 +190,7 @@ const SortOrderDropdown = styled.select``; interface PipelineStepProps { before: number | undefined; after: number | undefined; - step: string; + step: React.ReactNode; } /** diff --git a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx index a2f59daa162..d7fae2a3267 100644 --- a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx @@ -1,8 +1,10 @@ +import { Fragment } from "react"; + /** * A set of names, for generating unambiguous abbreviations. */ class NameSet { - private readonly abbreviations = new Map(); + private readonly abbreviations = new Map(); constructor(readonly names: string[]) { const qnames = names.map(parseName); @@ -14,7 +16,7 @@ class NameSet { }); } - public getAbbreviation(name: string) { + public getAbbreviation(name: string): React.ReactNode { return this.abbreviations.get(name) ?? name; } } @@ -84,7 +86,7 @@ class TrieNode { interface VisitResult { node: TrieNode; - abbreviate: () => string; + abbreviate: () => React.ReactNode; } class TrieBuilder { @@ -117,20 +119,28 @@ class TrieBuilder { return { node: trieNode, abbreviate: () => { - let result = ""; + const result: React.ReactNode[] = []; if (prefix != null) { - result += prefix.abbreviate(); - result += "::"; + result.push(prefix.abbreviate()); + result.push("::"); } - result += qname.name; + result.push(qname.name); if (args != null) { - result += "<"; + result.push("<"); if (trieNodeBeforeArgs.children.size === 1) { - result += "..."; + result.push("..."); } else { - result += args.map((arg) => arg.abbreviate()).join(","); + let first = true; + for (const arg of args) { + result.push(arg.abbreviate()); + if (first) { + first = false; + } else { + result.push(","); + } + } } - result += ">"; + result.push(">"); } return result; }, @@ -140,12 +150,31 @@ class TrieBuilder { const nameTokenRegex = /\b[^ ]+::[^ (]+\b/g; -export function abbreviateRASteps(steps: string[]): string[] { +export function abbreviateRASteps(steps: string[]): React.ReactNode[] { const nameTokens = steps.flatMap((step) => Array.from(step.matchAll(nameTokenRegex)).map((tok) => tok[0]), ); const nameSet = new NameSet(nameTokens); - return steps.map((step) => - step.replace(nameTokenRegex, (match) => nameSet.getAbbreviation(match)), - ); + return steps.map((step, index) => { + const matches = Array.from(step.matchAll(nameTokenRegex)); + const result: React.ReactNode[] = []; + for (let i = 0; i < matches.length; i++) { + const match = matches[i]; + const before = step.slice( + i === 0 ? 0 : matches[i - 1].index + matches[i - 1][0].length, + match.index, + ); + result.push(before); + result.push(nameSet.getAbbreviation(match[0])); + } + result.push( + matches.length === 0 + ? step + : step.slice( + matches[matches.length - 1].index + + matches[matches.length - 1][0].length, + ), + ); + return {result}; + }); } From d50a445a58b64479fe78d99a32bcb24bf33815e3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 13 Nov 2024 14:45:43 +0100 Subject: [PATCH 3/7] WIP on coloring RA names --- .../compare-performance/RAPrettyPrinter.tsx | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx index d7fae2a3267..ad3c22c08d4 100644 --- a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx @@ -1,4 +1,5 @@ import { Fragment } from "react"; +import { styled } from "styled-components"; /** * A set of names, for generating unambiguous abbreviations. @@ -12,7 +13,7 @@ class NameSet { qnames .map((qname) => builder.visitQName(qname)) .forEach((r, index) => { - this.abbreviations.set(names[index], r.abbreviate()); + this.abbreviations.set(names[index], r.abbreviate(true)); }); } @@ -86,7 +87,7 @@ class TrieNode { interface VisitResult { node: TrieNode; - abbreviate: () => React.ReactNode; + abbreviate: (isRoot?: boolean) => React.ReactNode; } class TrieBuilder { @@ -118,13 +119,21 @@ class TrieBuilder { } return { node: trieNode, - abbreviate: () => { + abbreviate: (isRoot = false) => { const result: React.ReactNode[] = []; if (prefix != null) { result.push(prefix.abbreviate()); result.push("::"); } - result.push(qname.name); + const { name } = qname; + const hash = name.indexOf("#"); + if (hash !== -1 && isRoot) { + const shortName = name.substring(0, hash); + result.push({shortName}); + result.push(name.substring(hash)); + } else { + result.push(isRoot ? {name} : name); + } if (args != null) { result.push("<"); if (trieNodeBeforeArgs.children.size === 1) { @@ -148,6 +157,17 @@ class TrieBuilder { } } +/** Span enclosing an entire qualified name. */ +const QNameSpan = styled.span` + color: var(--vscode-disabledForeground); +`; + +/** Span enclosing the innermost identifier, e.g. the `foo` in `A::B::foo#abc` */ +const IdentifierSpan = styled.span` + /* color: #4078f2; */ + color: var(--vscode-foreground); +`; + const nameTokenRegex = /\b[^ ]+::[^ (]+\b/g; export function abbreviateRASteps(steps: string[]): React.ReactNode[] { @@ -165,7 +185,7 @@ export function abbreviateRASteps(steps: string[]): React.ReactNode[] { match.index, ); result.push(before); - result.push(nameSet.getAbbreviation(match[0])); + result.push({nameSet.getAbbreviation(match[0])}); } result.push( matches.length === 0 From 56ababcddabefe02a56baa60262fc62daf106b85 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 14 Nov 2024 10:23:58 +0100 Subject: [PATCH 4/7] More work on nice syntax highlighting --- .../compare-performance/RAPrettyPrinter.tsx | 77 +++++++++++++------ 1 file changed, 53 insertions(+), 24 deletions(-) diff --git a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx index ad3c22c08d4..010523194e8 100644 --- a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx @@ -157,44 +157,73 @@ class TrieBuilder { } } -/** Span enclosing an entire qualified name. */ +/** + * Span enclosing an entire qualified name. + * + * Can be used to gray out uninteresting parts of the name, though this looks worse than expected. + */ const QNameSpan = styled.span` - color: var(--vscode-disabledForeground); + /* color: var(--vscode-disabledForeground); */ `; /** Span enclosing the innermost identifier, e.g. the `foo` in `A::B::foo#abc` */ const IdentifierSpan = styled.span` - /* color: #4078f2; */ - color: var(--vscode-foreground); + font-weight: 600; +`; + +/** Span enclosing keywords such as `JOIN` and `WITH`. */ +const KeywordSpan = styled.span` + font-weight: 500; `; -const nameTokenRegex = /\b[^ ]+::[^ (]+\b/g; +const nameTokenRegex = /\b[^ (]+\b/g; + +function traverseMatches( + text: string, + regex: RegExp, + callbacks: { + onMatch: (match: RegExpMatchArray) => void; + onText: (text: string) => void; + }, +) { + const matches = Array.from(text.matchAll(regex)); + let lastIndex = 0; + for (const match of matches) { + const before = text.substring(lastIndex, match.index); + if (before !== "") { + callbacks.onText(before); + } + callbacks.onMatch(match); + lastIndex = match.index + match[0].length; + } + const after = text.substring(lastIndex); + if (after !== "") { + callbacks.onText(after); + } +} export function abbreviateRASteps(steps: string[]): React.ReactNode[] { const nameTokens = steps.flatMap((step) => Array.from(step.matchAll(nameTokenRegex)).map((tok) => tok[0]), ); - const nameSet = new NameSet(nameTokens); + const nameSet = new NameSet(nameTokens.filter((name) => name.includes("::"))); return steps.map((step, index) => { - const matches = Array.from(step.matchAll(nameTokenRegex)); const result: React.ReactNode[] = []; - for (let i = 0; i < matches.length; i++) { - const match = matches[i]; - const before = step.slice( - i === 0 ? 0 : matches[i - 1].index + matches[i - 1][0].length, - match.index, - ); - result.push(before); - result.push({nameSet.getAbbreviation(match[0])}); - } - result.push( - matches.length === 0 - ? step - : step.slice( - matches[matches.length - 1].index + - matches[matches.length - 1][0].length, - ), - ); + traverseMatches(step, nameTokenRegex, { + onMatch(match) { + const text = match[0]; + if (text.includes("::")) { + result.push({nameSet.getAbbreviation(text)}); + } else if (/[A-Z]+/.test(text)) { + result.push({text}); + } else { + result.push(match[0]); + } + }, + onText(text) { + result.push(text); + }, + }); return {result}; }); } From 7de0bf6eaefc1e851104dcaaf959fc1a3153a1ac Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 14 Nov 2024 10:30:32 +0100 Subject: [PATCH 5/7] Apply a background color to the pipeline header rows --- .../compare-performance/ComparePerformance.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx b/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx index c3664ff11af..4e759f888f2 100644 --- a/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx @@ -216,6 +216,10 @@ function PipelineStep(props: PipelineStepProps) { ); } +const HeaderTR = styled.tr` + background-color: var(--vscode-sideBar-background); +`; + interface HighLevelStatsProps { before: PredicateInfo; after: PredicateInfo; @@ -229,13 +233,13 @@ function HighLevelStats(props: HighLevelStatsProps) { before.evaluationCount > 1 || after.evaluationCount > 1; return ( <> - + {hasBefore ? "Before" : ""} {hasAfter ? "After" : ""} {hasBefore && hasAfter ? "Delta" : ""} Stats - + {showEvaluationCount && ( ) { - + Before After Delta Predicate - +
{rows.map((row) => ( @@ -433,7 +437,7 @@ export function ComparePerformance(_: Record) { row.after.pipelines, ).map(({ name, first, second }, pipelineIndex) => ( - + {first != null && "Before"} {second != null && "After"} @@ -448,7 +452,7 @@ export function ComparePerformance(_: Record) { ? " (before)" : ""} - + {abbreviateRASteps(first?.steps ?? second!.steps).map( (step, index) => ( Date: Thu, 14 Nov 2024 11:07:43 +0100 Subject: [PATCH 6/7] Make "..." clickable to reveal abbreviated name --- .../compare-performance/RAPrettyPrinter.tsx | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx index 010523194e8..d02ef1e9c3a 100644 --- a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx @@ -1,4 +1,4 @@ -import { Fragment } from "react"; +import { Fragment, useState } from "react"; import { styled } from "styled-components"; /** @@ -29,6 +29,21 @@ interface QualifiedName { args?: QualifiedName[]; } +function qnameToString(name: QualifiedName): string { + const parts: string[] = []; + if (name.prefix != null) { + parts.push(qnameToString(name.prefix)); + parts.push("::"); + } + parts.push(name.name); + if (name.args != null && name.args.length > 0) { + parts.push("<"); + parts.push(name.args.map(qnameToString).join(",")); + parts.push(">"); + } + return parts.join(""); +} + function tokeniseName(text: string) { return Array.from(text.matchAll(/:+|<|>|,|"[^"]+"|`[^`]+`|[^:<>,"`]+/g)); } @@ -137,7 +152,10 @@ class TrieBuilder { if (args != null) { result.push("<"); if (trieNodeBeforeArgs.children.size === 1) { - result.push("..."); + const argsText = qname + .args!.map((arg) => qnameToString(arg)) + .join(","); + result.push({argsText}); } else { let first = true; for (const arg of args) { @@ -157,6 +175,30 @@ class TrieBuilder { } } +const ExpandableTextButton = styled.button` + background: none; + border: none; + cursor: pointer; + padding: 0; + color: inherit; + &:hover { + background-color: rgba(128, 128, 128, 0.2); + } +`; + +interface ExpandableNamePartProps { + children: React.ReactNode; +} + +function ExpandableNamePart(props: ExpandableNamePartProps) { + const [isExpanded, setExpanded] = useState(false); + return ( + setExpanded(!isExpanded)}> + {isExpanded ? props.children : "..."} + + ); +} + /** * Span enclosing an entire qualified name. * From 6de1b2f0d55ddb8a92e1116826efeef79bb789c9 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 14 Nov 2024 11:40:31 +0100 Subject: [PATCH 7/7] Also abbreviate RA names in predicate overview --- .../view/compare-performance/ComparePerformance.tsx | 8 +++++--- .../src/view/compare-performance/RAPrettyPrinter.tsx | 12 +++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx b/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx index 4e759f888f2..74107a40374 100644 --- a/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/ComparePerformance.tsx @@ -12,7 +12,7 @@ import type { import { formatDecimal } from "../../common/number"; import { styled } from "styled-components"; import { Codicon, ViewTitle, WarningBox } from "../common"; -import { abbreviateRASteps } from "./RAPrettyPrinter"; +import { abbreviateRANames, abbreviateRASteps } from "./RAPrettyPrinter"; const enum AbsentReason { NotSeen = "NotSeen", @@ -364,6 +364,8 @@ export function ComparePerformance(_: Record) { totalDiff += row.diff; } + const rowNames = abbreviateRANames(rows.map((row) => row.name)); + return ( <> Performance comparison @@ -406,7 +408,7 @@ export function ComparePerformance(_: Record) { - {rows.map((row) => ( + {rows.map((row, rowIndex) => ( ) { {renderAbsoluteValue(row.before)} {renderAbsoluteValue(row.after)} {renderDelta(row.diff)} - {row.name} + {rowNames[rowIndex]} {expandedPredicates.has(row.name) && ( <> diff --git a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx index d02ef1e9c3a..39038a8e520 100644 --- a/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx +++ b/extensions/ql-vscode/src/view/compare-performance/RAPrettyPrinter.tsx @@ -193,7 +193,12 @@ interface ExpandableNamePartProps { function ExpandableNamePart(props: ExpandableNamePartProps) { const [isExpanded, setExpanded] = useState(false); return ( - setExpanded(!isExpanded)}> + { + setExpanded(!isExpanded); + event.stopPropagation(); + }} + > {isExpanded ? props.children : "..."} ); @@ -269,3 +274,8 @@ export function abbreviateRASteps(steps: string[]): React.ReactNode[] { return {result}; }); } + +export function abbreviateRANames(names: string[]): React.ReactNode[] { + const nameSet = new NameSet(names); + return names.map((name) => nameSet.getAbbreviation(name)); +}