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

Unbounded memory growth in Flutter application - due to missing old-space GC #43770

Closed
mkustermann opened this issue Oct 13, 2020 · 5 comments
Closed
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. P1 A high priority bug; for example, a single project is unusable or has many test failures type-performance Issue relates to performance or code size

Comments

@mkustermann
Copy link
Member

The following flutter application runs an animation. When building each frame we do new memory allocations and make older objects unreachable. The live memory at any given point in time should be <100 MB, though the process will eventually OOM due to unbounded memory growth:

import 'package:flutter/animation.dart';                                                                                                                                                                                                                                                                                     
import 'package:flutter/material.dart';                                                                                                                                                                                                                                                                                      
                                                                                                                                                                                                                                                                                                                             
main() {                                                                                                                                                                                                                                                                                                                     
  final sim = AllocationSimulation();                                                                                                                                                                                                                                                                                        
  runApp(LogoApp(sim));                                                                                                                                                                                                                                                                                                      
}                                                                                                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                                                                                                             
class LogoApp extends StatefulWidget {                                                                                                                                                                                                                                                                                       
  final AllocationSimulation sim;                                                                                                                                                                                                                                                                                            
  LogoApp(this.sim);                                                                                                                                                                                                                                                                                                         
  LogoAppState createState() => LogoAppState(sim);                                                                                                                                                                                                                                                                           
}                                                                                                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                                                                                                             
class LogoAppState extends State<LogoApp> with SingleTickerProviderStateMixin {                                                                                                                                                                                                                                              
  final AllocationSimulation sim;                                                                                                                                                                                                                                                                                            
  Animation<double> animation;                                                                                                                                                                                                                                                                                               
  AnimationController controller;                                                                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                                                                                                             
  LogoAppState(this.sim);                                                                                                                                                                                                                                                                                                    
                                                                                                                                                                                                                                                                                                                             
  void initState() {                                                                                                                                                                                                                                                                                                         
    super.initState();                                                                                                                                                                                                                                                                                                       
    controller =                                                                                                                                                                                                                                                                                                             
        AnimationController(duration: const Duration(seconds: 2), vsync: this);                                                                                                                                                                                                                                              
    animation = CurvedAnimation(parent: controller, curve: Curves.easeIn)                                                                                                                                                                                                                                                    
      ..addStatusListener((status) {                                                                                                                                                                                                                                                                                         
        if (status == AnimationStatus.completed) {                                                                                                                                                                                                                                                                           
          controller.reverse();                                                                                                                                                                                                                                                                                              
        } else if (status == AnimationStatus.dismissed) {                                                                                                                                                                                                                                                                    
          controller.forward();                                                                                                                                                                                                                                                                                              
        }                                                                                                                                                                                                                                                                                                                    
      });                                                                                                                                                                                                                                                                                                                    
    controller.forward();                                                                                                                                                                                                                                                                                                    
  }                                                                                                                                                                                                                                                                                                                          
                                                                                                                                                                                                                                                                                                                             
  Widget build(BuildContext context) => AnimatedLogo(sim, animation: animation);                                                                                                                                                                                                                                             

  void dispose() {
    controller.dispose();
    super.dispose();
  }
}

class AnimatedLogo extends AnimatedWidget {
  static final _sizeTween = Tween<double>(begin: 0, end: 300);

  final AllocationSimulation sim;

  AnimatedLogo(this.sim, {Key key, Animation<double> animation})
      : super(key: key, listenable: animation);

  Widget build(BuildContext context) {
    // Produce some garbage.
    sim.onFrameBuild();

    final animation = listenable as Animation<double>;
    return Center(
      child: Container(
        margin: EdgeInsets.symmetric(vertical: 10),
        height: _sizeTween.evaluate(animation),
        width: _sizeTween.evaluate(animation),
        child: FlutterLogo(),
      ),
    );
  }
}

class AllocationSimulation {
  // Array of this size is around 1 kb.
  static const int chunkSize = 128;

  // Make enough arrays to exhaust new-space limit.
  static const int chunks = 17 * 1024;

  // For each frame: Replace this many old arrays with new arrays.
  static const int iterateCount = 8 * 1024;

  List root, current;

  AllocationSimulation() {
    root = List(chunkSize);
    var last = root;
    for (int i = 0; i < chunks; ++i) {
      final nc = List(chunkSize);
      last[0] = nc;
      last = nc;
    }
    current = root;
  }

  void onFrameBuild() {
    for (int i = 0; i < iterateCount; ++i) {
      if (current[0] == null) current = root;
      final old = current[0];
      final replacement = List(chunkSize)..[0] = old[0];
      current[0] = replacement;
      current = replacement;
    }
  }
}

Most likely this happens due to missing start of old space collections during the Dart_NotifyIdle calls from flutter engine:
Screen Shot 2020-10-13 at 14 09 35

Here we can see that every Dart_NotifyIdle will cause a scavenge that almost exhausts the 16 ms limit - which might be why we fail to start old space collection.

=> We should ensure to have tests for our heuristics - effectively adding regression tests for such bugs.

/cc @rmacnak-google Maybe you could take a look?
/cc @dnfield Since it's related to idle notification. Do you have any suggestions how to add memory tests on flutter side?

@mkustermann mkustermann added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. P1 A high priority bug; for example, a single project is unusable or has many test failures type-performance Issue relates to performance or code size labels Oct 13, 2020
@dnfield
Copy link
Contributor

dnfield commented Oct 13, 2020

We can add a test like this to the devicelab, which can track memory usage over the life of the process.

@rmacnak-google
Copy link
Contributor

@mkustermann This example behaves differently on my device, with a mixture of idle and non-idle GCs, including idle old-space GC. Could you send me the timeline captured from your device?

NotifyIdle will already ignore the deadline if old-space is sufficiently full, so I suspect something else has gone wrong; perhaps the threshold has been corrupted or the force-growth bit got stuck.

@mkustermann
Copy link
Member Author

@rmacnak-google For ease of running I use Flutter's desktop embedding:

% flutter run --local-engine-src-path=$HOME/fe --local-engine=host_profile  --profile  -d macos

It's sometimes a little hard to trigger this behavior. I've rebased now and got one trace. In that trace it seems we are triggering old space collections. Though we still cause unbounded memory growth. It might be that the old space collector cannot keep pace.

unbounded-memory-growth.json.gz

@rmacnak-google
Copy link
Contributor

It looks like the write-barrier elimination bug you found was effectively creating a memory leak by its corruption in this example. I observed unbounded growth even though old-space GC was happening, the heap not shrinking even with forced compaction, and heap snapshots also reporting the heap as all used. Some added Dart code observed cycles in the linked list. With the write-barrier fix patched in, I no longer observe the unbounded growth and the cycles disappear. Both before and after the writer-barrier fix, I don't observe the scenario where old-space GC doesn't happen, so that seems like a separate issue.

@mkustermann
Copy link
Member Author

@rmacnak-google The example was flakily crashing for me, which lead me to the bug in write-barrier elimination. I've tried now with the fix and it seems on my Mac the memory growth doesn't happen anymore so it's probably indeed due to the bug.

@mkustermann mkustermann self-assigned this Oct 15, 2020
dart-bot pushed a commit that referenced this issue Oct 22, 2020
If there is a GC-triggering instruction between array allocation and
store into array we cannot omit the barrier. This worked for stores
where the receiver is a direct array allocation, but not if it's a phi.

Closes #43786
Closes #43770

Change-Id: I28de29cf85842a9d3ae3189bb8e6426969fe4279
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167570
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
dart-bot pushed a commit that referenced this issue Oct 27, 2020
If there is a GC-triggering instruction between array allocation and
store into array we cannot omit the barrier. This worked for stores
where the receiver is a direct array allocation, but not if it's a phi.

Closes #43786
Closes #43770

Change-Id: I28de29cf85842a9d3ae3189bb8e6426969fe4279
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167570
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. P1 A high priority bug; for example, a single project is unusable or has many test failures type-performance Issue relates to performance or code size
Projects
None yet
Development

No branches or pull requests

3 participants