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

feat: small-string optimization #9895

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomberek
Copy link
Contributor

@tomberek tomberek commented Jan 31, 2024

If a string can fit into a Value, without a context, then pack it in. This avoids extra allocation and clean up for common strings.

Motivation

Speed up Nix's string performance, reduce memory usage.

  • memory: 98%
  • cpu: ~same
    • I expected a bigger improvement as this avoids nearly 1M allocations. Very likely my C++-fu could be better.

Context

Looking at various approaches: small-strings, symbol table usage, storing strings as a list of pointers for efficient concat without allocation.

It is easy to violate GC assumptions when doing things like this, so I expect to go through a few iterations and plenty of testing.

ping: @pennae

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

If a string can fit into a Value, without a context, then pack it in.
This avoids extra allocation and clean up for common strings.
@tomberek tomberek requested a review from edolstra as a code owner January 31, 2024 21:07
@tomberek tomberek marked this pull request as draft February 1, 2024 01:17
@pennae
Copy link
Contributor

pennae commented Feb 1, 2024

this works and does reduce memory use, but it makes some hot paths in the code much more expensive and increases the number of instructions executed overall. it may be more productive to reduce the size of Value to a single tagged pointer, reusing most bits of what is now the type field to store non-gc pointers and the type tag. that too would be very intrusive though and itself not likely to speed things up all that much :(

@Mic92
Copy link
Member

Mic92 commented Feb 2, 2024

What does memory: 98% mean? Do you save 2% of memory? And in what benchmark?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-03-04-nix-team-meeting-minute-130/40830/1

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

Successfully merging this pull request may close these issues.

4 participants