Skip to content

Commit

Permalink
ui/packages/shared/profile/IcicleGraphArrow: Improve coloring and cle…
Browse files Browse the repository at this point in the history 
…an up code
  • Loading branch information
@metalmatze
metalmatze committed Jul 4, 2023
1 parent d21ff97 commit 35eb5c8
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 252 deletions.
16 changes: 8 additions & 8 deletions ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/ColorStackLegend.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,11 @@ const ColorStackLegend = ({
navigateTo,
compareMode = false,
}: Props): React.JSX.Element => {
if (mappingColors === undefined) {
return <></>;
}

const [colorProfileName] = useUserPreference<string>(
USER_PREFERENCES.FLAMEGRAPH_COLOR_PROFILE.key
);
const [currentSearchString, setSearchString] = useURLState({param: 'search_string', navigateTo});

if (Object.entries(mappingColors).length === 0) {
return <></>;
}

const stackColorArray = useMemo(() => {
return Object.entries(mappingColors).sort(([featureA], [featureB]) => {
if (featureA === EVERYTHING_ELSE) {
Expand All @@ -57,6 +49,14 @@ const ColorStackLegend = ({
});
}, [mappingColors]);

if (mappingColors === undefined) {
return <></>;
}

if (Object.entries(mappingColors).length === 0) {
return <></>;
}

if (colorProfileName === 'default' || compareMode) {
return <></>;
}
Expand Down
40 changes: 20 additions & 20 deletions ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/IcicleGraphNodes.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,8 @@ import {Table} from 'apache-arrow';
import cx from 'classnames';

import {useKeyDown} from '@parca/components';
import {
EVERYTHING_ELSE,
selectBinaries,
setHoveringRow,
useAppDispatch,
useAppSelector,
} from '@parca/store';
import {getLastItem, isSearchMatch, scaleLinear} from '@parca/utilities';
import {selectBinaries, setHoveringRow, useAppDispatch, useAppSelector} from '@parca/store';
import {isSearchMatch, scaleLinear} from '@parca/utilities';

import {
FIELD_CHILDREN,
Expand All @@ -33,6 +27,7 @@ import {
FIELD_FUNCTION_NAME,
FIELD_MAPPING_FILE,
} from './index';
import useNodeColor from './useNodeColor';
import {nodeLabel} from './utils';

export const RowHeight = 26;
Expand All @@ -53,6 +48,7 @@ interface IcicleGraphNodesProps {
xScale: (value: bigint) => number;
searchString?: string;
sortBy: string;
darkMode: boolean;
compareMode: boolean;
}

Expand All @@ -71,6 +67,7 @@ export const IcicleGraphNodes = React.memo(function IcicleGraphNodesNoMemo({
curPath,
sortBy,
searchString,
darkMode,
compareMode,
}: IcicleGraphNodesProps): React.JSX.Element {
const cumulatives = table.getChild(FIELD_CUMULATIVE);
Expand Down Expand Up @@ -109,6 +106,7 @@ export const IcicleGraphNodes = React.memo(function IcicleGraphNodesNoMemo({
xScale={xScale}
sortBy={sortBy}
searchString={searchString}
darkMode={darkMode}
compareMode={compareMode}
/>
);
Expand Down Expand Up @@ -138,6 +136,7 @@ interface IcicleNodeProps {
isRoot?: boolean;
searchString?: string;
sortBy: string;
darkMode: boolean;
compareMode: boolean;
}

Expand Down Expand Up @@ -168,6 +167,7 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
isRoot = false,
searchString,
sortBy,
darkMode,
compareMode,
}: IcicleNodeProps): React.JSX.Element {
const {isShiftDown} = useKeyDown();
Expand All @@ -182,6 +182,7 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
const mappingFile: string | null = mappingColumn?.get(row);
const functionName: string | null = functionNameColumn?.get(row);
const cumulative: bigint = cumulativeColumn?.get(row);
const diff: bigint | null = diffColumn?.get(row);
const children: number[] = Array.from(table.getChild(FIELD_CHILDREN)?.get(row) ?? []);

// TODO: Maybe it's better to pass down the sorter function as prop instead of figuring this out here.
Expand All @@ -205,7 +206,6 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
children.sort((a, b) => {
const aDiff: bigint | null = diffColumn?.get(a);
const bDiff: bigint | null = diffColumn?.get(b);
// TODO: Double check this sorting actually makes sense, when coloring is back
if (aDiff !== null && bDiff !== null) {
return Number(bDiff - aDiff);
}
Expand All @@ -222,8 +222,15 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
}

const binaries = useAppSelector(selectBinaries);
// const {isShiftDown} = useKeyDown();
// const colorResult = useNodeColor({table, row, compareMode});
const colorResult = useNodeColor({
isDarkMode: darkMode,
compareMode,
cumulative,
diff,
mappingColors,
mappingFile,
functionName,
});
const name = useMemo(() => {
return isRoot ? 'root' : nodeLabel(table, row, binaries.length > 1);
}, [table, row, isRoot, binaries]);
Expand Down Expand Up @@ -263,14 +270,6 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
dispatch(setHoveringRow(undefined));
};

// To get the color we first check if the function name starts with 'runtime'.
// If it does, we color it as runtime. Otherwise, we check the mapping file.
// If there is no mapping file, we color it as 'everything else'.
const color =
functionName?.startsWith('runtime') === true
? mappingColors.runtime
: mappingColors[getLastItem(mappingFile ?? '') ?? EVERYTHING_ELSE];

return (
<>
<g
Expand All @@ -288,7 +287,7 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
width={width}
height={height}
style={{
fill: color,
fill: colorResult,
}}
className={cx('stroke-white dark:stroke-gray-700', {
'opacity-50': isHighlightEnabled && !isHighlighted,
Expand Down Expand Up @@ -319,6 +318,7 @@ export const IcicleNode = React.memo(function IcicleNodeNoMemo({
setCurPath={setCurPath}
searchString={searchString}
sortBy={sortBy}
darkMode={darkMode}
compareMode={compareMode}
/>
)}
Expand Down
52 changes: 32 additions & 20 deletions ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,34 +82,45 @@ export const IcicleGraphArrow = memo(function IcicleGraphArrow({
selectQueryParam('compare_a') === 'true' && selectQueryParam('compare_b') === 'true';

const mappings = useMemo(() => {
// Reading the mappings from the dictionary that contains all mapping strings.
// Read the mappings from the dictionary that contains all mapping strings.
// This is great, as might only have a dozen or so mappings,
// and don't need to read through all the rows (potentially thousands).
const mappingsDict: Vector<Dictionary> | null = table.getChild(FIELD_MAPPING_FILE);
const mappings = Array.from(mappingsDict?.data.values() ?? [])
.map((mapping): string[] => {
const dict = mapping.dictionary;
const len = dict?.data.length ?? 0;
const values: string[] = [];
for (let i = 0; i <= len; i++) {
// Read the value and only append the binaries last part - binary name
values.push(getLastItem(dict?.get(i)) ?? '');
}
return values;
})
.flat();
const mappings =
mappingsDict?.data
.map(mapping => {
if (mapping.dictionary == null) {
return [];
}
const len = mapping.dictionary.length;
const entries: string[] = [];
for (let i = 0; i < len; i++) {
entries.push(getLastItem(mapping.dictionary.get(i)) ?? '');
}
return entries;
})
.flat() ?? [];

// We add a EVERYTHING ELSE mapping to the list.
mappings.push('');

// We look through the function names to find out if there's a runtime function.
// Again, we only read through the dictionary, which is much faster than reading through all the rows.
// We stop as soon as we find a runtime function.
const functionNamesDict: Vector<Dictionary> | null = table.getChild(FIELD_FUNCTION_NAME);
// TODO: There must be a better way to do this. Somehow read the function name dictionary rather than iterating over all rows.
for (let i = 0; i < table.numRows; i++) {
const fn: string | null = functionNamesDict?.get(i);
if (fn?.startsWith('runtime') === true) {
mappings.push('runtime');
break;
functionNamesDict?.data.forEach(fn => {
if (fn.dictionary == null) {
return;
}
}
const len = fn.dictionary.length;
for (let i = 0; i < len; i++) {
const fn: string | null = functionNamesDict?.get(i);
if (fn?.startsWith('runtime') === true) {
mappings.push('runtime');
break;
}
}
});

// We sort the mappings alphabetically to make sure that the order is always the same.
mappings.sort((a, b) => a.localeCompare(b));
Expand Down Expand Up @@ -196,6 +207,7 @@ export const IcicleGraphArrow = memo(function IcicleGraphArrow({
isRoot={true}
searchString={currentSearchString}
sortBy={sortBy}
darkMode={isDarkMode}
compareMode={compareMode}
/>
</g>
Expand Down
117 changes: 0 additions & 117 deletions ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/useColoredGraph.ts
View file

This file was deleted.

Loading

0 comments on commit 35eb5c8

Please sign in to comment.