Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flamegraph renders incorrectly on successive renders with different data #1766

Closed
Tracked by #1806
dgieselaar opened this issue Jul 29, 2022 · 4 comments · May be fixed by #1781
Closed
Tracked by #1806

Flamegraph renders incorrectly on successive renders with different data #1766

dgieselaar opened this issue Jul 29, 2022 · 4 comments · May be fixed by #1781
Labels
bug Something isn't working :flamegraph Flame/Icicle chart related issues

Comments

@dgieselaar
Copy link
Member

(copied from @jbcrail's issue in a private repo

Describe the bug

When the time range is updated for a flamegraph, we fetch the respective data; then the flamegraph will re-render without a page refresh. In the common case where the data before and after are different, then the flamegraph will render incorrectly (text will be offset from the expected rectangle bounds, colors will not applied to the rectangles, etc). At least in Firefox, we see an error like this:

WebGL warning: drawArraysInstanced: Instance fetch requires 15054, but attribs only supply 12822.

What's interesting is that the left number (15054) represents the length of the position array before the time range was updated, whereas 12822 is the array length after fetching.

To Reproduce

Steps to reproduce the behavior:

  1. Go to Flamegraphs section
  2. Select a time range larger than the existing range
  3. See error

See these screenshots that reproduce the behavior:

  • Flamegraph before updating time range

30 minutes

  • Flamegraph after updating time range (1st time)

24 hours

  • Flamegraph after updating time range (2nd time)

7 days

Expected behavior

The flamegraph should always rendering correctly when the time range is updated. No WebGL warnings should be seen in the browser console.

Additional context

I created a sample test case from the existing storybook story to demonstrate the issue. I have two different columnar datasets and the example toggles between the datasets every 10 seconds.

flamechart

/*
 * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
 * or more contributor license agreements. Licensed under the Elastic License
 * 2.0 and the Server Side Public License, v 1; you may not use this file except
 * in compliance with, at your election, the Elastic License 2.0 or the Server
 * Side Public License, v 1.
 */

import { action } from '@storybook/addon-actions';
import React, { useEffect, useState } from 'react';

import { Chart, ColumnarViewModel, Datum, Flame, Settings, PartialTheme, ControlProviderCallback } from '@elastic/charts';
import columnarMock from '@elastic/charts/src/mocks/hierarchical/cpu_profile_tree_mock_columnar.json';
import { getRandomNumberGenerator } from '@elastic/charts/src/mocks/utils';

import { useBaseTheme } from '../../use_base_theme';

const pseudoRandom = getRandomNumberGenerator('a_seed');

const paletteColorBrewerCat12 = [
  [141, 211, 199],
  [255, 255, 179],
  [190, 186, 218],
  [251, 128, 114],
  [128, 177, 211],
  [253, 180, 98],
  [179, 222, 105],
  [252, 205, 229],
  [217, 217, 217],
  [188, 128, 189],
  [204, 235, 197],
  [255, 237, 111],
];

function generateColorsForLabels(labels: any[]): number[] {
  return labels.flatMap(() => [...paletteColorBrewerCat12[pseudoRandom(0, 11)].map((c) => c / 255), 1]);
}

const largeDataset: ColumnarViewModel = (() => {
  const labels = columnarMock.label;
  const value = new Float64Array(columnarMock.value);
  const position = new Float32Array(columnarMock.position);
  const size = new Float32Array(columnarMock.size);

  return {
    label: labels.map((index: number) => columnarMock.dictionary[index]),
    value: value,
    color: new Float32Array(generateColorsForLabels(labels)),
    position0: position,
    position1: position,
    size0: size,
    size1: size,
  };
})();

const smallDataset: ColumnarViewModel = (() => {
  const labels = ["root", "kernel", "libxul.so", "libxul.so"];
  const value = [1428, 1428, 1099, 329];
  const position = [0, 0.67, 0, 0.33, 0, 0, 0.769607, 0];
  const size = [1, 1, 0.769607, 0.230393];

  return {
    label: labels,
    value: new Float64Array(value),
    color: new Float32Array(generateColorsForLabels(labels)),
    position0: new Float32Array(position),
    position1: new Float32Array(position),
    size0: new Float32Array(size),
    size1: new Float32Array(size),
  };
})();

export const Example = (
  // should check why it's not a good idea; in the meantime:
  // eslint-disable-next-line unicorn/no-object-as-default-parameter
  props?: { controlProviderCallback: ControlProviderCallback },
) => {
  const [columnarData, setColumnarData] = useState(largeDataset);
  const [seconds, setSeconds] = useState(0);

  const onElementListeners = {
    onElementClick: action('onElementClick'),
    onElementOver: action('onElementOver'),
    onElementOut: action('onElementOut'),
  };

  const theme: PartialTheme = {
    chartMargins: { top: 0, left: 0, bottom: 0, right: 0 },
    chartPaddings: { left: 0, right: 0, top: 0, bottom: 0 },
  };

  useEffect(() => {
    const interval = setInterval(() => {
      setSeconds(seconds => seconds + 1);
    }, 1000);
    return () => clearInterval(interval);
  }, []);

  useEffect(() => {
    if (seconds > 0 && seconds % 10 === 0) {
      if ((seconds / 10) % 2 == 0) {
        setColumnarData(largeDataset)
      } else {
        setColumnarData(smallDataset)
      }
    }
  }, [seconds]);

  return (
    <Chart>
      <Settings theme={theme} baseTheme={useBaseTheme()} {...onElementListeners} />
      <Flame
        id="spec_1"
        columnarData={columnarData}
        valueAccessor={(d: Datum) => d.value as number}
        valueFormatter={(value) => `${value}`}
        animation={{ duration: 500 }}
        controlProviderCallback={props?.controlProviderCallback ?? (() => { })}
      />
    </Chart>
  );
};

Example.parameters = {
  background: { default: 'white' },
};
@dgieselaar dgieselaar added the bug Something isn't working label Jul 29, 2022
@markov00 markov00 added the :flamegraph Flame/Icicle chart related issues label Aug 4, 2022
@monfera monfera assigned monfera and unassigned monfera Aug 4, 2022
@monfera
Copy link
Contributor

monfera commented Aug 5, 2022

Thanks for the detailed report! We're looking into it. In the meantime, to avoid being blocked, would a fresh instantiation of the chart work? Ie. it gets a new key, so the old chart and its resources are removed from the DOM tree and a fresh one gets inserted

@dgieselaar
Copy link
Member Author

@monfera I'm adding a key prop here: elastic/kibana#138498. IMHO that's good enough for now.

@timductive
Copy link
Member

referencing original issue: https://github.com/elastic/prodfiler/issues/2460

@markov00
Copy link
Member

Closing as a workaround is already used. Please reopen in case you need an elastic-charts side fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working :flamegraph Flame/Icicle chart related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants