diff options
author | Aspen Smith <root@gws.fyi> | 2024-02-13T19·59-0500 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-02-21T20·53+0000 |
commit | 5e31096154d32bf128f8eb172f84ca3b7b968bc8 (patch) | |
tree | af991a2428954e98599a061a49a08d80abdadac3 /tvix/eval/src/vm | |
parent | 77a6eb4f512b6d8ae94b82677d8efe3eacf8cfa8 (diff) |
feat(tvix/eval): Store string context alongside data r/7591
Previously, Nix strings were represented as a Box (within Value) pointing to a tuple of an optional context, and another Box pointing to the actual string allocation itself. This is pretty inefficient, both in terms of memory usage (we use 48 whole bytes for a None context!) and in terms of the extra indirection required to get at the actual data. It was necessary, however, because with native Rust DSTs if we had something like `struct NixString(Option<NixContext>, BStr)` we could only pass around *fat* pointers to that value (with the length in the pointer) and that'd make Value need to be bigger (which is a waste of both memory and cache space, since that memory would be unused for all other Values). Instead, this commit implements *manual* allocation of a packed string representation, with the length *in the allocation* as a field past the context. This requires a big old pile of unsafe Rust, but the payoff is clear: hello outpath time: [882.18 ms 897.16 ms 911.23 ms] change: [-15.143% -13.819% -12.500%] (p = 0.00 < 0.05) Performance has improved. Fortunately this change can be localized entirely within value/string.rs, since we were abstracting things out nicely. Change-Id: Ibf56dd16c9c503884f64facbb7f0ac596463efb6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10852 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: aspen <root@gws.fyi>
Diffstat (limited to 'tvix/eval/src/vm')
-rw-r--r-- | tvix/eval/src/vm/generators.rs | 2 | ||||
-rw-r--r-- | tvix/eval/src/vm/mod.rs | 10 |
2 files changed, 5 insertions, 7 deletions
diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index 2a2710dc34af..4dacdef0dd08 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/tvix/eval/src/vm/generators.rs @@ -607,7 +607,7 @@ pub async fn request_string_coerce( kind: CoercionKind, ) -> Result<NixString, CatchableErrorKind> { match val { - Value::String(s) => Ok(*s), + Value::String(s) => Ok(s), _ => match co.yield_(VMRequest::StringCoerce(val, kind)).await { VMResponse::Value(Value::Catchable(c)) => Err(*c), VMResponse::Value(value) => Ok(value diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs index 32f0d0456f14..c10b79cd992e 100644 --- a/tvix/eval/src/vm/mod.rs +++ b/tvix/eval/src/vm/mod.rs @@ -1000,9 +1000,7 @@ where } self.stack - .push(Value::String(Box::new(NixString::new_context_from( - context, out, - )))); + .push(Value::String(NixString::new_context_from(context, out))); Ok(()) } @@ -1263,7 +1261,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> { Err(c) => Value::Catchable(Box::new(c)), } } - (Value::String(s1), Value::String(s2)) => Value::String(Box::new(s1.concat(&s2))), + (Value::String(s1), Value::String(s2)) => Value::String(s1.concat(&s2)), (Value::String(s1), v) => generators::request_string_coerce( &co, v, @@ -1274,7 +1272,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> { }, ) .await - .map(|s2| Value::String(Box::new(s1.concat(&s2)))) + .map(|s2| Value::String(s1.concat(&s2))) .into(), (a @ Value::Integer(_), b) | (a @ Value::Float(_), b) => arithmetic_op!(&a, &b, +)?, (a, b) => { @@ -1297,7 +1295,7 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> { ) .await; match (r1, r2) { - (Ok(s1), Ok(s2)) => Value::String(Box::new(s1.concat(&s2))), + (Ok(s1), Ok(s2)) => Value::String(s1.concat(&s2)), (Err(c), _) => return Ok(Value::from(c)), (_, Err(c)) => return Ok(Value::from(c)), } |