-
Notifications
You must be signed in to change notification settings - Fork 365
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
wip: memory optimizations #2503
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thoughtpolice
force-pushed
the
aseipp/push-mwwotvxyruwp
branch
6 times, most recently
from
November 2, 2023 16:41
990ddb8
to
7f44bce
Compare
thoughtpolice
force-pushed
the
aseipp/push-mwwotvxyruwp
branch
4 times, most recently
from
November 4, 2023 15:48
6c2032d
to
7f759e4
Compare
arxanas
reviewed
Nov 4, 2023
thoughtpolice
force-pushed
the
aseipp/push-mwwotvxyruwp
branch
from
November 5, 2023 04:31
7f759e4
to
d601d94
Compare
Summary: NIH, but will have meaning in follow up commits. Signed-off-by: Austin Seipp <[email protected]> Change-Id: I2b61bf13e5e33381712dfb27370f616d
Signed-off-by: Austin Seipp <[email protected]> Change-Id: I187fa569b49c626357d316333eeb55e6
Summary: This module is 100% abstract except for this single export. No other API is supported at this time, by design. Signed-off-by: Austin Seipp <[email protected]> Change-Id: I0316dcf3ec85972336056e8fdca86f33
Signed-off-by: Austin Seipp <[email protected]> Change-Id: I522b80f0fcfe3173c1412cdd9f49d127
Summary: One of the more useful features of mimalloc. In the future this interface could be expanded to support printing to a callback, so that it could be triggered and dumped e.g. on debugging endpoints for longer running processes, when we get there. However, it's not enough to just add it to the `main` module; not only would it be ugly and overspecific, it should be reusable so other custom `jj` commands can opt-in to statistics trivially as well. Users should be strongly encouraged to do this -- enable mimalloc and stat tracking -- since that's what upstream will now work and test with; but, of course, all things are optional. Thus, we expose an API from the `cli_utils` module for use with `cli_util::CliRunner::add_global_args`, but ultimately it's up to the client to call it. Signed-off-by: Austin Seipp <[email protected]> Change-Id: I6abe4b962bbe7c62ebca97be630b750c
Summary: As suggested by Yuya. This is actually a huge win in heap allocations for repositories with a lot of files; according to the bucket statistics from mimalloc, on gecko-dev, a repo of ~350,000 files, this reduces the number of small alloctions (8, 16, or 32 byte size class) from nearly ~4 million to just over ~200k, a 20x allocation reduction, with a total peak heap size reduction of over 50MiB. Signed-off-by: Austin Seipp <[email protected]> Change-Id: I4014145d740367ecec5be32c6b9e2de8
Signed-off-by: Austin Seipp <[email protected]> Change-Id: I2007f35f645b890ab94467614c6094c0
4 tasks
thoughtpolice
force-pushed
the
aseipp/push-mwwotvxyruwp
branch
from
November 6, 2023 04:46
d601d94
to
560ca85
Compare
6 tasks
Abandoning this since the mimalloc changes are handled elsewhere and the optimizations are now obsolete thanks to many changes from Yuya in particular. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From last night, so it doesn't get lost, and for CI testing (since branch pushes don't cause one, apparently). I'll probably open a new PR for splitting up these changes.