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 after adding touchCallback and checkToShowDot #1644

Open
av4625 opened this issue Apr 30, 2024 · 4 comments
Open

Memory leaks after adding touchCallback and checkToShowDot #1644

av4625 opened this issue Apr 30, 2024 · 4 comments

Comments

@av4625
Copy link

av4625 commented Apr 30, 2024

Don't make a duplicate issue.

I'm not sure if its the same as this issue as it only happened after I used the callbacks in the title and the example in that issue doesn't use them: #1106

If it is the same I can close this and record it over there.

Describe the bug

I have multiple charts that all show relating data. The data in this example has around 1500 points per line and I have tested up to 5 lines, so is fairly large. When I hover over one I wanted to show "spots" on the others.

To do this I added a touchCallback that saves the hovered spot locations in a riverpod NotifierProvider. All charts watch this and then in their checkToShowDot it checks for the spots that relate to the hover data in the NotifierProvider and then draws them.

I have noticed that the flutter app started using lots of RAM after I added this hover functionality. I have seen over 2Gb and climbing in apples activity monitor. The app starts between 200-300mb. I opened the memory page of devtools and it shows that there where millions of FlSpot objects and they climb in numbers fast when hovering on a chart. It suffers from frame jank and gets slower and slower the more you hover until it becomes unusable.

It is possible I'm misusing the API as I'm new to Dart/Flutter.

To Reproduce

This is one of the 4 charts in the video (the middle one of the 3 on the right side). All 4 charts are almost identical to this.

class SpeedChart extends Chart {
  const SpeedChart({super.key, super.showXAxis});

  @override
  ChartType get chartType => ChartType.speed;

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final HoverLocations hoverData = ref.watch(hoverProvider);
    final RunData runData = ref.watch(runDataProvider);

    if (runData.isEmpty)
    {
      return const SizedBox.shrink();
    }

    return LineChart(
      LineChartData(
        lineTouchData: LineTouchData(
          handleBuiltInTouches: true,
          touchSpotThreshold: 20,
          touchCallback: (final FlTouchEvent ev, final LineTouchResponse? res) =>
            notifyHoverLocations(ref, ev, res),
        ),
        gridData: const FlGridData(show: false),
        titlesData: getTitles(false, "Speed (MPH)"),
        borderData: FlBorderData(show: false),
        lineBarsData: _buildLineData(hoverData, runData),
      ),
    );
  }

  List<LineChartBarData> _buildLineData(
    final HoverLocations hoverData,
    final RunData runData) {
    final List<LineChartBarData> lineData = <LineChartBarData>[];

    for (final (index, run) in runData.runNames.indexed) {
      if (runData.getIsActive(run)) {
        lineData.add(LineChartBarData(
          isCurved: false,
          barWidth: 1.5,
          color: (index < chartLineColours.length) ? chartLineColours[index] : null,
          dotData: FlDotData(
            show: true,
            checkToShowDot: (final FlSpot spot, final LineChartBarData barData) =>
              showDotsForHover(hoverData, spot, barData),
          ),
          spots: () {
            final List<double> distances = runData.getDistances(run);
            final List<double> speeds = runData.getSpeeds(run);

            return List<FlSpot>.generate(
              distances.length,
              (index) {
                return FlSpot(distances[index], speeds[index]);
              }
            );
          }()
        ));
      }
    }

    return lineData;
  }
}

These two functions are in a parent class:

void notifyHoverLocations(
  final WidgetRef ref,
  final FlTouchEvent ev,
  final LineTouchResponse? res) {
  if (!ev.isInterestedForInteractions)
  {
    ref.read(hoverProvider.notifier).setHoveredChart(null);
  }
  else
  {
    for (final TouchLineBarSpot hoverSpot in res?.lineBarSpots ?? []) {
      ref.read(hoverProvider.notifier).addOrUpdateLineHover(
        hoverSpot.bar.color?.value ?? 0, hoverSpot.x, hoverSpot.spotIndex);
    }

    ref.read(hoverProvider.notifier).setHoveredChart(chartType);
  }
}

bool showDotsForHover(
  final HoverLocations hoverData,
  final FlSpot spot,
  final LineChartBarData barData) {
  if ((hoverData.currentHoveredChart ?? chartType) != chartType &&
    hoverData.colourHasHover(barData.color?.value ?? 0) &&
    spot.x == hoverData.getHoverLocation(barData.color?.value ?? 0).$1) {
    return true;
  }

  return false;
}

Before adding the two callbacks I mention above I just had this and the FlSpots never change from 27093 for 5 lines:

lineTouchData: const LineTouchData(
          handleBuiltInTouches: true,
          touchSpotThreshold: 50,
        )

Screenshots

Video of the charts and how they work (you can see there is major frame jank at times):
https://github.com/imaNNeo/fl_chart/assets/19472253/e42fbd6f-311e-435f-9c4f-a7cd1b5db3b0

A snap of the number of FlSpot objects:
snap1

Hovered for about 10 seconds and took a snap of the number of FlSpot objects again:
snap2

Versions

Flutter 3.19.5 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 300451adae (5 weeks ago) • 2024-03-27 21:54:07 -0500
Engine • revision e76c956498
Tools • Dart 3.3.3 • DevTools 2.31.1
fl_chart: ^0.67.0
@av4625
Copy link
Author

av4625 commented May 2, 2024

I have done some further testing and removing checkToShowDot does not fix the memory leak.
But removing my touchCallback does fix the memory leak.

I also tried keeping my callbacks for checkToShowDot and touchCallback but setting handleBuiltInTouches to false, this also doesn't fix the memory leak.

@av4625
Copy link
Author

av4625 commented May 2, 2024

After even more testing:

I found if the callback I provide for touchCallback contains ref which is a WidgetRef for riverpod I get this issue.

I changed the touchCallback to this (I know its dumb code but I just wanted the bare minimum to make the issue happen):

touchCallback: (final FlTouchEvent ev, final LineTouchResponse? res) {
  if (ref != null) {
  }
}

I get the memory leak issue. None of my providers contain any FlSpots so I am very confused at how this is happening.

@av4625
Copy link
Author

av4625 commented May 2, 2024

I have done a memory snap shot and have discovered that it is more than just FlSpots that are growing. First snap shot is on first draw of the charts and second snap shot is after a 5 second hover back and forth:
Screenshot 2024-05-02 at 23 35 53

There are 4 charts with 2 lines each in this example.

@av4625
Copy link
Author

av4625 commented May 3, 2024

Here is a full main.dart to reproduce the issue. It is much simpler with only having one chart:

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:fl_chart/fl_chart.dart';

void main() {
  runApp(const ProviderScope(child: MyApp()));
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: SafeArea(
        child: Scaffold(
          body: Column(
            children: [
              Expanded(child: Chart(),),
              Expanded(child: Chart(),),
            ]
          )
        )
      ),
      theme: ThemeData(useMaterial3: true),
    );
  }
}

final testProvider = StateProvider<int>((ref) => 0);

class Chart extends ConsumerWidget {
  Chart({super.key});

  static const int _numberOfLines = 3;
  static const int _numberOfDataPoints = 1500;

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final int index = ref.watch(testProvider);

    return LineChart(
      LineChartData(
        lineTouchData: LineTouchData(
          handleBuiltInTouches: false,
          touchSpotThreshold: 20,
          touchCallback: (final FlTouchEvent ev, final LineTouchResponse? res) {
            ref.read(testProvider.notifier).state = res?.lineBarSpots?.first.spotIndex ?? 0;
          },
        ),
        gridData: const FlGridData(show: false),
        borderData: FlBorderData(show: false),
        lineBarsData: _buildLineData(index),
      ),
    );
  }

  List<LineChartBarData> _buildLineData(final int index) {
    final List<LineChartBarData> lineData = <LineChartBarData>[];

    for (int i = 0; i < _numberOfLines; ++i) {
      lineData.add(LineChartBarData(
        showingIndicators: <int>[index],
        dotData: FlDotData(show: false),
        spots: () {
          return List<FlSpot>.generate(
            _numberOfDataPoints,
            (index) {
              return FlSpot(index.toDouble(), index.toDouble());
            }
          );
        }()
      ));
    }

    return lineData;
  }
}

You will see that there are 3 lines with 1500 spots each and when you start this app it starts with 4501 FlSpot objects. Then if you hover this just grows and grows and doesn't stop (other objects grow as well).

Required dependencies:

fl_chart: ^0.67.0
flutter_riverpod: ^2.5.1

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

No branches or pull requests

1 participant