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

Memory leaks with continue updated input data #69

Closed
shenenya opened this issue Oct 31, 2019 · 26 comments
Closed

Memory leaks with continue updated input data #69

shenenya opened this issue Oct 31, 2019 · 26 comments
Labels

Comments

@shenenya
Copy link

I found these is memory leaks problem with continue updated input data. The browser will crash after a while. Is it caused by react-vega, vega-embed or vega? Thanks.

BTW. The problem can be repeated by clicking the 'Update data' button in the example frequently.

@domoritz
Copy link
Member

Is this a Vegas or react Vega issue? Could you help us find out?

@shenenya
Copy link
Author

I checked out codes of react-vega. It release old view with finalize function. I also look at vega-embed. But I do not find possible reason by now. I think it maybe caused by async or vega's implementation of finalize function.

@domoritz
Copy link
Member

Can you try to reproduce this issue with just Vega?

@shenenya
Copy link
Author

shenenya commented Nov 1, 2019

Ok. By now, I found that when we use onNewView in react-vega and update state in the function called. The memory leaks will be serious. Even I try to release the view with finalize function. Could you help me to explain why and what we could do? Thanks.

@domoritz
Copy link
Member

domoritz commented Nov 1, 2019

Can you see whether the same happens with just Vega?

@shenenya
Copy link
Author

shenenya commented Nov 15, 2019

I reproduced the problem with ReactVegaLiteDemo as follows:

  1. install react-timeout to stories
  2. change ReactVegaLiteDemo.jsx to follows:
import React from "react";
import { action } from "@storybook/addon-actions";
import { Vega, createClassFromSpec } from "../../react-vega/src";
import ReactTimeout from "react-timeout";

const data1 = {
  myData: [
    { a: "A", b: 20 },
    { a: "B", b: 34 },
    { a: "C", b: 55 },
    { a: "D", b: 19 },
    { a: "E", b: 40 },
    { a: "F", b: 34 },
    { a: "G", b: 91 },
    { a: "H", b: 78 },
    { a: "I", b: 25 },
    { a: "A", b: 20 },
    { a: "B", b: 34 },
    { a: "C", b: 55 },
    { a: "D", b: 19 },
    { a: "E", b: 40 },
    { a: "F", b: 34 },
    { a: "G", b: 91 },
    { a: "H", b: 78 },
    { a: "I", b: 25 }
  ]
};

const data2 = {
  myData: [
    { a: "A", b: 28 },
    { a: "B", b: 55 },
    { a: "C", b: 43 },
    { a: "D", b: 91 },
    { a: "E", b: 81 },
    { a: "F", b: 53 },
    { a: "G", b: 19 },
    { a: "H", b: 87 },
    { a: "I", b: 52 }
  ]
};

const spec1 = {
  $schema: "https://vega.github.io/schema/vega-lite/v4.0.0-beta.10.json",
  data: { name: "myData" },
  description: "A simple bar chart with embedded data.",
  encoding: {
    x: { field: "a", type: "ordinal" },
    y: { field: "b", type: "quantitative" }
  },
  mark: "bar"
};

const spec2 = {
  data: { name: "myData" },
  description: "A simple bar chart with embedded data.",
  encoding: {
    x: { field: "b", type: "quantitative" },
    y: { field: "a", type: "ordinal" }
  },
  mark: "bar"
};

const BarChart = createClassFromSpec({ mode: "vega-lite", spec: spec1 });

const code1 = `<VegaLite data={this.state.data} spec={this.state.spec} />`;

const code2 = `const BarChart = ReactVegaLite.createClassFromLiteSpec(spec1);
<BarChart data={this.state.data} />`;

class Demo extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      data: data1,
      info: "",
      spec: spec1
    };

    this.handleHover = this.handleHover.bind(this);
    this.handleToggleSpec = this.handleToggleSpec.bind(this);
    this.handleUpdateData = this.handleUpdateData.bind(this);
    this.handlers = { hover: this.handleHover };
  }

  setTimer = () => {
    var timer = function() {
      this.handleUpdateData();
      this.handleToggleSpec();
      this.timerHandle = this.props.setTimeout(timer.bind(this), 1);
    };
    this.timerHandle = this.props.setTimeout(timer.bind(this), 1);
  };

  componentDidMount = () => {
    this.setTimer();
  };

  handleHover(...args) {
    action("hover", {
      limit: 5
    })(args);
    this.setState({
      info: JSON.stringify(args)
    });
  }

  handleToggleSpec() {
    const { spec } = this.state;
    action("toggle spec")(spec);
    if (spec === spec1) {
      this.setState({ spec: spec2 });
    } else {
      this.setState({ spec: spec1 });
    }
  }

  handleUpdateData() {
    const { data } = this.state;
    action("update data")(data);
    if (data === data1) {
      this.setState({ data: data2 });
    } else if (data === data2) {
      this.setState({ data: data1 });
    }
  }

  render() {
    const { data, spec, info } = this.state;

    return (
      <div>
        <div style={{ float: "right" }}>
          <iframe
            title="star"
            src="https://ghbtns.com/github-btn.html?user=vega&repo=react-vega&type=star&count=true"
            frameBorder="0"
            scrolling="0"
            width="100px"
            height="20px"
          />
        </div>
        <button type="button" onClick={this.handleToggleSpec}>
          Toggle Spec
        </button>
        <button type="button" onClick={this.handleUpdateData}>
          Update data
        </button>
        <h3>
          <code>&lt;VegaLite&gt;</code> React Component
        </h3>
        Will recompile when spec changes and update when data changes.
        <pre>{code1}</pre>
        <Vega data={data} spec={spec} />
        <h3>
          <code>ReactVegaLite.createClassFromLiteSpec()</code>
        </h3>
        Use the given spec to create a reusable component.
        <pre>{code2}</pre>
        <BarChart data={data} />
        {info}
      </div>
    );
  }
}

export default ReactTimeout(Demo);
  1. Check the system memory costs. You could find the memory used by browser keep increasing.

@domoritz
Copy link
Member

Can you reproduce the same with just Vega (no react)?

Note that Vega uses a reactive data flow and therefore has to keep track of some data. However, this doesn’t mean that memory usage continuously increases.

@shenenya
Copy link
Author

shenenya commented Nov 16, 2019

Hi. I reproduce the same problem with just vega-lite as follows.

<!DOCTYPE html>
<head>
  <meta charset="utf-8" />
  <script src="https://cdn.jsdelivr.net/npm/vega@5"></script>
  <script src="https://cdn.jsdelivr.net/npm/vega-lite@4.0.0-beta.2"></script>
  <script src="https://cdn.jsdelivr.net/npm/vega-embed@5"></script>
</head>

<body>
  <div id="vis"></div>

  <script>
    const spec1 = {
      $schema: "https://vega.github.io/schema/vega-lite/v3.json",
      description: "A simple bar chart with embedded data.",
      width: 360,
      data: {
        values: [
          { a: "A", b: 28 },
          { a: "B", b: 55 },
          { a: "C", b: 43 },
          { a: "D", b: 91 },
          { a: "E", b: 81 },
          { a: "F", b: 53 },
          { a: "G", b: 19 },
          { a: "H", b: 87 },
          { a: "I", b: 52 }
        ]
      },
      mark: "bar",
      encoding: {
        x: { field: "a", type: "ordinal" },
        y: { field: "b", type: "quantitative" },
        tooltip: { field: "b", type: "quantitative" }
      }
    };
    const spec2 = {
      $schema: "https://vega.github.io/schema/vega-lite/v3.json",
      description: "A simple bar chart with embedded data.",
      width: 500,
      data: {
        values: [
          { a: "A", b: 33 },
          { a: "B", b: 32 },
          { a: "C", b: 16 },
          { a: "D", b: 28 },
          { a: "E", b: 35 },
          { a: "F", b: 81 },
          { a: "G", b: 90 },
          { a: "H", b: 10 },
          { a: "I", b: 5 }
        ]
      },
      mark: "bar",
      encoding: {
        x: { field: "a", type: "ordinal" },
        y: { field: "b", type: "quantitative" },
        tooltip: { field: "b", type: "quantitative" }
      }
    };
    let spec = spec1;
    function myFunction() {
      setInterval(function() {
        if (spec === spec1) spec = spec2;
        else spec = spec1;

        vegaEmbed("#vis", spec)
          .then()
          .catch(console.warn);
      }, 1000);
    }
  </script>

  <button onclick="myFunction()">Update</button>
</body>

@domoritz
Copy link
Member

domoritz commented Nov 17, 2019

The embed example is fixed by using .finalize(). See vega/vega-embed#270

React Vega already uses finalize so I'm not sure yet what's happening here

.

@shenenya
Copy link
Author

I tried react-vega. It has the problem.

@domoritz
Copy link
Member

Moving discussion to vega/vega-embed#270.

@kristw
Copy link
Member

kristw commented Nov 17, 2019

@eeyshen I notice that in the react-vega example, you setTimeout every 1ms while the vega-lite one is every 1000ms.

Is that intentional?

@shenenya
Copy link
Author

I also tried vega-lite with 1ms. The problem will show itself more quick.

@domoritz domoritz added the bug label Nov 27, 2019
@domoritz
Copy link
Member

To fix this, please call result.finalize() in Embed 6.2.0.

@shenenya
Copy link
Author

shenenya commented Nov 27, 2019

Do you mean what have done in VegaEmbed.tsx:

  clearView() {
    this.modifyView(view => {
      view.finalize();
    });
    this.viewPromise = undefined;

    return this;
  }

@domoritz
Copy link
Member

No, we should fix react Vega to call this. Users shouldn’t need to call finalize directly.

@shenenya
Copy link
Author

shenenya commented Dec 2, 2019

@domoritz Hi, when will the next release of react-vega be published with the bug fixed? Thanks.

@domoritz
Copy link
Member

domoritz commented Dec 2, 2019

I don't have the cycles to add the fix. Maybe you can send a or @kristw may have time.

@shenenya
Copy link
Author

shenenya commented Dec 4, 2019

@kristw Do you have time to fix this bug with Vega-Embed 6.2 and release a new version?

@shenenya
Copy link
Author

shenenya commented Dec 7, 2019

As Vega, Vega-Lite, Vega-Embed upgrades, is there some plan for react-vega upgrade? Thanks.

@kristw
Copy link
Member

kristw commented Dec 10, 2019

I am on vacation with limited access. However, pull requests are welcome.

@kristw
Copy link
Member

kristw commented Mar 22, 2020

Is this resolved?

@shenenya
Copy link
Author

I believe so.

@kristw kristw closed this as completed Mar 22, 2020
@shenenya
Copy link
Author

I believe the problem appear again. The screenshot below show the memory profile of the example above.

<!DOCTYPE html>
<head>
  <meta charset="utf-8" />
  <script src="https://cdn.jsdelivr.net/npm/vega@5"></script>
  <script src="https://cdn.jsdelivr.net/npm/vega-lite@4.12.1"></script>
  <script src="https://cdn.jsdelivr.net/npm/vega-embed@6"></script>
</head>

<body>
  <div id="vis"></div>

  <script>
    const spec1 = {
      $schema: "https://vega.github.io/schema/vega-lite/v4.json",
      description: "A simple bar chart with embedded data.",
      width: 360,
      data: {
        values: [
          { a: "A", b: 28 },
          { a: "B", b: 55 },
          { a: "C", b: 43 },
          { a: "D", b: 91 },
          { a: "E", b: 81 },
          { a: "F", b: 53 },
          { a: "G", b: 19 },
          { a: "H", b: 87 },
          { a: "I", b: 52 }
        ]
      },
      mark: "bar",
      encoding: {
        x: { field: "a", type: "ordinal" },
        y: { field: "b", type: "quantitative" },
        tooltip: { field: "b", type: "quantitative" }
      }
    };
    const spec2 = {
      $schema: "https://vega.github.io/schema/vega-lite/v4.json",
      description: "A simple bar chart with embedded data.",
      width: 500,
      data: {
        values: [
          { a: "A", b: 33 },
          { a: "B", b: 32 },
          { a: "C", b: 16 },
          { a: "D", b: 28 },
          { a: "E", b: 35 },
          { a: "F", b: 81 },
          { a: "G", b: 90 },
          { a: "H", b: 10 },
          { a: "I", b: 5 }
        ]
      },
      mark: "bar",
      encoding: {
        x: { field: "a", type: "ordinal" },
        y: { field: "b", type: "quantitative" },
        tooltip: { field: "b", type: "quantitative" }
      }
    };
    let spec = spec1;
    function myFunction() {
      setInterval(function() {
        if (spec === spec1) spec = spec2;
        else spec = spec1;

        vegaEmbed("#vis", spec)
          .then()
          .catch(console.warn);
      }, 1000);
    }
  </script>

  <button onclick="myFunction()">Update</button>
</body>

截屏2020-05-18 上午9 17 01

@domoritz
Copy link
Member

I responded in the related Vega embed issue.

@shenenya
Copy link
Author

shenenya commented May 18, 2020

I found the memory leak happens again.

I reproduced the problem with ReactVegaLiteDemo as follows:

  1. install react-timeout to stories
  2. change ReactVegaLiteDemo.jsx to follows:
import React from "react";
import { action } from "@storybook/addon-actions";
import { Vega, createClassFromSpec } from "../../react-vega/src";
import ReactTimeout from "react-timeout";

const data1 = {
  myData: [
    { a: "A", b: 20 },
    { a: "B", b: 34 },
    { a: "C", b: 55 },
    { a: "D", b: 19 },
    { a: "E", b: 40 },
    { a: "F", b: 34 },
    { a: "G", b: 91 },
    { a: "H", b: 78 },
    { a: "I", b: 25 },
    { a: "A", b: 20 },
    { a: "B", b: 34 },
    { a: "C", b: 55 },
    { a: "D", b: 19 },
    { a: "E", b: 40 },
    { a: "F", b: 34 },
    { a: "G", b: 91 },
    { a: "H", b: 78 },
    { a: "I", b: 25 }
  ]
};

const data2 = {
  myData: [
    { a: "A", b: 28 },
    { a: "B", b: 55 },
    { a: "C", b: 43 },
    { a: "D", b: 91 },
    { a: "E", b: 81 },
    { a: "F", b: 53 },
    { a: "G", b: 19 },
    { a: "H", b: 87 },
    { a: "I", b: 52 }
  ]
};

const spec1 = {
  $schema: "https://vega.github.io/schema/vega-lite/v4.0.0-beta.10.json",
  data: { name: "myData" },
  description: "A simple bar chart with embedded data.",
  encoding: {
    x: { field: "a", type: "ordinal" },
    y: { field: "b", type: "quantitative" }
  },
  mark: "bar"
};

const spec2 = {
  data: { name: "myData" },
  description: "A simple bar chart with embedded data.",
  encoding: {
    x: { field: "b", type: "quantitative" },
    y: { field: "a", type: "ordinal" }
  },
  mark: "bar"
};

const BarChart = createClassFromSpec({ mode: "vega-lite", spec: spec1 });

const code1 = `<VegaLite data={this.state.data} spec={this.state.spec} />`;

const code2 = `const BarChart = ReactVegaLite.createClassFromLiteSpec(spec1);
<BarChart data={this.state.data} />`;

class Demo extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      data: data1,
      info: "",
      spec: spec1
    };

    this.handleHover = this.handleHover.bind(this);
    this.handleToggleSpec = this.handleToggleSpec.bind(this);
    this.handleUpdateData = this.handleUpdateData.bind(this);
    this.handlers = { hover: this.handleHover };
  }

  setTimer = () => {
    var timer = function() {
      this.handleUpdateData();
      this.handleToggleSpec();
      this.timerHandle = this.props.setTimeout(timer.bind(this), 1);
    };
    this.timerHandle = this.props.setTimeout(timer.bind(this), 1);
  };

  componentDidMount = () => {
    this.setTimer();
  };

  handleHover(...args) {
    action("hover", {
      limit: 5
    })(args);
    this.setState({
      info: JSON.stringify(args)
    });
  }

  handleToggleSpec() {
    const { spec } = this.state;
    action("toggle spec")(spec);
    if (spec === spec1) {
      this.setState({ spec: spec2 });
    } else {
      this.setState({ spec: spec1 });
    }
  }

  handleUpdateData() {
    const { data } = this.state;
    action("update data")(data);
    if (data === data1) {
      this.setState({ data: data2 });
    } else if (data === data2) {
      this.setState({ data: data1 });
    }
  }

  render() {
    const { data, spec, info } = this.state;

    return (
      <div>
        <div style={{ float: "right" }}>
          <iframe
            title="star"
            src="https://ghbtns.com/github-btn.html?user=vega&repo=react-vega&type=star&count=true"
            frameBorder="0"
            scrolling="0"
            width="100px"
            height="20px"
          />
        </div>
        <button type="button" onClick={this.handleToggleSpec}>
          Toggle Spec
        </button>
        <button type="button" onClick={this.handleUpdateData}>
          Update data
        </button>
        <h3>
          <code>&lt;VegaLite&gt;</code> React Component
        </h3>
        Will recompile when spec changes and update when data changes.
        <pre>{code1}</pre>
        <Vega data={data} spec={spec} />
        <h3>
          <code>ReactVegaLite.createClassFromLiteSpec()</code>
        </h3>
        Use the given spec to create a reusable component.
        <pre>{code2}</pre>
        <BarChart data={data} />
        {info}
      </div>
    );
  }
}

export default ReactTimeout(Demo);
  1. Check the system memory costs. You could find the memory used by browser keep increasing.

截屏2020-05-18 下午4 41 13

截屏2020-05-18 下午4 44 07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants