imaNNeo / fl_chart

FL Chart is a highly customizable Flutter chart library that supports Line Chart, Bar Chart, Pie Chart, Scatter Chart, and Radar Chart.
https://flchart.dev
MIT License
6.58k stars 1.71k forks source link

Memory leaking with changing spot list #1106

Open Clon1998 opened 1 year ago

Clon1998 commented 1 year ago

Describe the bug When chaning the complete FLSpot list instance/reference e.g. calculating it based on another list whenever the list changes the LineChart is leaking a ton of FlSpots

Here is a DevTool-measurement within seconds the Spots grows to 100k+ instances ( Example code used to measure can be found below)

Bildschirmfoto 2022-07-29 um 19 27 23

To Reproduce To debug the behavior I had a look at the Sample#10 which does not leak any memory. However, I edited and also managed to reproduce the FLSpot leaking I encountered in my own app. Instead of using an instance of final sinePoints=<FlSpot>[] (src) I am using a final sinPoints = <double>[]; list which I later map to FlSpots using the _convertToPlotSpots method. Also to verify that the issue is caused by the chart I replaced the LineChart widget with an Text('Test:${_convertToPlotSpots(sinPoints).last.x}'); widget that uses also the FlSpot list just to see if the leaking is caused by the code in _convertToPlotSpots which is not the case. Finaly I found a workaround for me by using a single instance of List<FlSpot>that does not change/only swapps elements using this code for the _convertToPlotSpots method:

// Workaround that uses again a fixed instance of List<FlSpot> and just replace the elements of it
List<FlSpot> l = [];
List<FlSpot> _convertToPlotSpots(List<double>? doubles) {
  if (doubles == null) return const [];
  List<double> sublist = doubles.sublist(max(0, doubles.length - 300));
  l.clear();
  double i = 0;
  l.addAll(sublist
      .map((e) => FlSpot(i++, e)));
  return l;
}

Sample#10 leaking Memory:

// Widget that leaks memory 
class LineChartSample10 extends StatefulWidget {
  const LineChartSample10({Key? key}) : super(key: key);

  @override
  _LineChartSample10State createState() => _LineChartSample10State();
}

class _LineChartSample10State extends State<LineChartSample10> {
  final Color sinColor = Colors.redAccent;
  final Color cosColor = Colors.blueAccent;

  final limitCount = 300;
  final sinPoints = <double>[];

  double xValue = 0;
  double step = 0.05;

  late Timer timer;

  @override
  void initState() {
    super.initState();
    timer = Timer.periodic(const Duration(milliseconds: 40), (timer) {
      if (sinPoints.length > 1200) sinPoints.removeAt(0);
      setState(() {
        sinPoints.add(math.sin(xValue));
      });
      xValue += step;
    });
  }

  @override
  Widget build(BuildContext context) {
    return sinPoints.isNotEmpty
        ? Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: [
              Text(
                'x: ${xValue.toStringAsFixed(1)}',
                style: const TextStyle(
                  color: Colors.black,
                  fontSize: 18,
                  fontWeight: FontWeight.bold,
                ),
              ),
              Text(
                'sin: ${sinPoints.last.toStringAsFixed(1)}',
                style: TextStyle(
                  color: sinColor,
                  fontSize: 18,
                  fontWeight: FontWeight.bold,
                ),
              ),
              const SizedBox(
                height: 12,
              ),
              SizedBox(
                width: 300,
                height: 300,
                child: _Chartextends(
                  sinPoints: sinPoints,
                ),
              )
            ],
          )
        : Container();
  }

  @override
  void dispose() {
    timer.cancel();
    super.dispose();
  }
}

class _Chartextends extends StatelessWidget {
  static const Color sinColor = Colors.redAccent;
  static const Color cosColor = Colors.blueAccent;

  final List<double> sinPoints;

  const _Chartextends({Key? key, required this.sinPoints}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    // Enable to test if its caused by the method and not by the LineChart
    // return Text('Test:${_convertToPlotSpots(sinPoints).last.x}');

    return LineChart(
      LineChartData(
        minY: -1,
        maxY: 1,
        lineTouchData: LineTouchData(enabled: false),
        clipData: FlClipData.all(),
        gridData: FlGridData(
          show: true,
          drawVerticalLine: false,
        ),
        lineBarsData: [
          sinLine(_convertToPlotSpots(sinPoints)),
        ],
        titlesData: FlTitlesData(
          show: false,
        ),
      ),
    );
  }

  LineChartBarData sinLine(List<FlSpot> points) {
    return LineChartBarData(
      spots: points,
      dotData: FlDotData(
        show: false,
      ),
      gradient: LinearGradient(
          colors: [sinColor.withOpacity(0), sinColor],
          begin: Alignment.centerLeft,
          end: Alignment.centerRight,
          stops: const [0.1, 1.0]),
      barWidth: 4,
      isCurved: false,
    );
  }

  LineChartBarData cosLine(List<FlSpot> points) {
    return LineChartBarData(
      spots: points,
      dotData: FlDotData(
        show: false,
      ),
      gradient: LinearGradient(
          colors: [sinColor.withOpacity(0), sinColor],
          begin: Alignment.centerLeft,
          end: Alignment.centerRight,
          stops: const [0.1, 1.0]),
      barWidth: 4,
      isCurved: false,
    );
  }
}

List<FlSpot> _convertToPlotSpots(List<double>? doubles) {
  if (doubles == null) return const [];
  List<double> sublist = doubles.sublist(max(0, doubles.length - 300));
  double i = 0;
  return sublist
      .map((e) => FlSpot(i++, e))
      .toList(growable: false);
}

Flutter 3.0.3 • channel stable • https://github.com/flutter/flutter.git Framework • revision 676cefaaff (5 weeks ago) • 2022-06-22 11:34:49 -0700 Engine • revision ffe7b86a1e Tools • Dart 2.17.5 • DevTools 2.12.2

fl_chart: ^0.55.0

FlorianArnould commented 1 year ago

I’m pretty sure it’s because of the static cache as I said here. The ‘LineChartHelper._cachedResults’ should generate the leak maintaining references to the lists and so the FlSpots.

Clon1998 commented 1 year ago

It might be the case I didn't look too much into this since I managed to find a workaround. However, I will have a look at the code you mentioned.

FlorianArnould commented 1 year ago

Sorry I’m on holidays and I don’t have the time nor the device to check this but could you retry your example having the memory leak but with calling a clear cache on the LineChartHelper each time you change the data values. Your will need to add this clearCache static method just clearing the _results map. If it is what I think it is, it should ‘fix’ your leak and we will need to refactor this. And already thanks for your help and early investigation !

AndroidDesigner commented 5 months ago

I’m pretty sure it’s because of the static cache as I said here. The ‘LineChartHelper._cachedResults’ should generate the leak maintaining references to the lists and so the FlSpots.

Dear @imaNNeo What is the solution?I see same problem in memory, so that while I close my chart screen, but memory filled by chart data is not cleared.

imaNNeo commented 5 months ago

I’m pretty sure it’s because of the static cache as I said here. The ‘LineChartHelper._cachedResults’ should generate the leak maintaining references to the lists and so the FlSpots.

Dear @imaNNeo What is the solution?I see same problem in memory, so that while I close my chart screen, but memory filled by chart data is not cleared.

Fixed in the main branch. I will notify you here when we released a new version.

imaNNeo commented 5 months ago

Fixed in 0.66.1, please test and let me know if the issue is fixed.

Thanks for your patience!

AndroidDesigner commented 5 months ago

Thank you dear @imaNNeo I will try it and let you know the result here.

ldy8070 commented 5 months ago

I'm using lineChartData, but I'm getting a Platform_enviroment error after upgrading the version. The platform currently used is web.

imaNNeo commented 5 months ago

I'm using lineChartData, but I'm getting a Platform_enviroment error after upgrading the version. The platform currently used is web.

This is another issue related to #1565 Please test the memory issue in the next release

imaNNeo commented 4 months ago

Guys, this memory issue should be resolved on 0.66.1. Can you please validate it in your apps?

NeariX67 commented 1 month ago

I'm on 0.68.0 and still have FlSpots taking up multiple Gigabytes after a few hours. Can't confirm if the cause is the same as described here. image

epaterlini commented 3 weeks ago

same problem here...I can solve the problem by not using the built-in "min" / "max" autocalculation and doing it myself. I will be more precise soon with some code.

wenjiasen commented 3 weeks ago

The root cause of this issue is that _cachedResults does not have a reliable cleanup mechanism. If you think providing a isEnableCache option is bad 1283 at least add a time limit or size limit?

wenjiasen commented 3 weeks ago

It seems that in 0.66.2, the cache was removed in the deletion of line_chart_data.dart, in this commit. However, there is still code using _cachedResults in the current project, and the OOM risk still exists.

imaNNeo commented 2 weeks ago

Thanks for sharing the information. As we don't have any quick solution to fix the issue, I will disable the caching for now. The trade off is that we need to calculate the min, max values based on the data (if it is not provided) in every frame. I wrote in the documentation that it is more performant if you provide these parameters.

Btw, for the final fix (resolve the performance issue if you haven't provided the parameters), we need to move the calculation logic into the renderer objects as they remain in the memory as long as widget exists. So this way, we don't need to calculate the values in every frame. We just keep and calculate them once the data is changed in the renderer objects that we have. I have created #1693 to allow you to follow up the further improvements