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

Spanned Value step 1: span all value cases #10042

Merged
merged 8 commits into from Aug 24, 2023

Conversation

sophiajt
Copy link
Member

Description

This doesn't really do much that the user could see, but it helps get us ready to do the steps of the refactor to split the span off of Value, so that values can be spanless. This allows us to have top-level values that can hold both a Value and a Span, without requiring that all values have them.

We expect to see significant memory reduction by removing so many unnecessary spans from values. For example, a table of 100,000 rows and 5 columns would have a savings of ~8megs in just spans that are almost always duplicated.

User-Facing Changes

Nothing yet

Tests + Formatting

After Submitting

@sophiajt sophiajt marked this pull request as draft August 18, 2023 04:48
@sophiajt
Copy link
Member Author

Making this draft so we don't accidentally land it. It should get a pretty good looking over while we do the refactor to make sure we aren't breaking things as we go.

@amtoine
Copy link
Member

amtoine commented Aug 18, 2023

can't wait 😋

@sophiajt
Copy link
Member Author

#10103 should land first

@sophiajt sophiajt marked this pull request as ready for review August 24, 2023 20:10
@sophiajt sophiajt merged commit 1e3e034 into nushell:main Aug 24, 2023
19 checks passed
@sophiajt sophiajt deleted the spanned_value_wip2 branch August 24, 2023 20:48
@fdncred fdncred added the pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes label Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants