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

Provide hints on the type of mappings provided to the Dart VM. #92120

Closed
chinmaygarde opened this issue Oct 19, 2021 · 9 comments
Closed

Provide hints on the type of mappings provided to the Dart VM. #92120

chinmaygarde opened this issue Oct 19, 2021 · 9 comments
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) customer: money (g3) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list perf: memory Performance issues related to memory

Comments

@chinmaygarde
Copy link
Member

chinmaygarde commented Oct 19, 2021

The Flutter Engine creates multiple mappings on Dart VM and Isolates initialization. However, not all regions in these mappings have the same usage and lifecycle requirements. Some are read-only while others are executable. Some are needed for the entire lifetime of the VM or Isolate while others can be freed immediately.

Ideally, the engine would have discrete mappings for each usage and precisely manage the lifecycle of each mapping on its own. However the number, size, and mechanism to generate these mappings is not fixed. This significantly complicates application packaging. These mappings are also internal implementation details in the VM.

The VM would like to provide advice to the operating system on how it intends to use various regions within the larger mappings. This is primarily done with platform specific calls like madvise. However, this is currently not possible today because VM doesn’t know how the mapping was created. All the engine gives the VM today is a pointer to the mapping and its size. With just this information, the VM cannot guarantee that the advice it gives the operating system is sound.

For instance, in the case of madvise and MADV_DONTNEED on Linux & Darwin, anonymous private mappings will be zero-filled. This will mess up subsequent uses of the mapping.

The engine already has knowledge about the types of mappings it gives the VM. This is done via subclasses of the pure virtual fml::Mapping in the engine. The engine and the VM should coordinate on a mechanism for tagging these mappings when they are provided to the the VM.

This approach should yield observable reductions in the memory usage of Flutter applications on platforms where the madvise call (or its platform specific implementation) is supported and implemented.

@chinmaygarde chinmaygarde added engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) perf: memory Performance issues related to memory labels Oct 19, 2021
@chinmaygarde
Copy link
Member Author

@rmacnak-google @a-siva: Let me know if I missed something from our conversation yesterday. cc @jason-simmons @zanderso who are looking into similar issues with engine managed snapshots.

@a-siva
Copy link
Contributor

a-siva commented Oct 20, 2021

The CL that @rmacnak-google has here https://dart-review.googlesource.com/c/sdk/+/216580 has demonstrated that it does reduce the resident memory size of the app process (see numbers in the CL).
We should try and get this landed for the next stable release, what is the suggested path forward.

@zanderso
Copy link
Member

It looks like the memory footprint savings are a bit under 10%. Do I have that right? It's a pretty nice chunk. There are only 6 working days before we start trying to cut the branch, so the schedule might be tight, but my suggestion would be that @jason-simmons continue on to this issue after #91382.

@a-siva
Copy link
Contributor

a-siva commented Oct 21, 2021

It looks like the memory footprint savings are a bit under 10%. Do I have that right? It's a pretty nice chunk. There are only 6 working days before we start trying to cut the branch, so the schedule might be tight, but my suggestion would be that @jason-simmons continue on to this issue after #91382.

The change itself is small, I think if we agree on the plumbing needed for the engine, we could land the Dart side of the change early next week and ensure it gets rolled in.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Oct 25, 2021
…e snapshot.

This region is mostly unused after loading, but it may be accessed again if
 - Isolate.spawn is invoke with isolate groups disabled
 - A secondary snapshot produced by splitting is loaded
 - An external typed data in the snapshot is accessed (usually a kernel file)
 - Likely other cases

Even if these cases did not exist, the region is often part of a shared library and so unable to be released independently.

madvise(DONT_NEED) on this region will cause the OS to release the memory in this region but keep the address space reservation and mapping. If it is touched again, it will be brought back in from the file. If it is not backed by a file, such as malloc memory, it will be brought back in as zeros and the program will likely fail.

TEST=ci
Bug: #44019
Bug: flutter/flutter#92120
Change-Id: I315a049b0f7d440e181d0a5e87fa6770a2fd4f79
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/216580
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
@chinmaygarde chinmaygarde added the P1 High-priority issues at the top of the work list label Oct 25, 2021
@a-siva
Copy link
Contributor

a-siva commented Oct 26, 2021

@zanderso @jason-simmons The commit https://dart-review.googlesource.com/c/sdk/+/216580 has rolled into the engine and I think the change to use it in the engine can now be done.

@jason-simmons
Copy link
Member

@rmacnak-google has a PR in progress: flutter/engine#29320

@zanderso
Copy link
Member

Is there anything left to do on this? Should we close it?

@a-siva
Copy link
Contributor

a-siva commented Nov 19, 2021

I believe we can close it.

@github-actions
Copy link

github-actions bot commented Dec 3, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) customer: money (g3) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list perf: memory Performance issues related to memory
Projects
None yet
Development

No branches or pull requests

6 participants