3 Commits

Author	SHA1	Message	Date
Ian Manske	8da27a1a09	Create Record type (#10103) ... # Description This PR creates a new `Record` type to reduce duplicate code and possibly bugs as well. (This is an edited version of #9648.) - `Record` implements `FromIterator` and `IntoIterator` and so can be iterated over or collected into. For example, this helps with conversions to and from (hash)maps. (Also, no more `cols.iter().zip(vals)`!) - `Record` has a `push(col, val)` function to help insure that the number of columns is equal to the number of values. I caught a few potential bugs thanks to this (e.g. in the `ls` command). - Finally, this PR also adds a `record!` macro that helps simplify record creation. It is used like so: ```rust record! { "key1" => some_value, "key2" => Value::string("text", span), "key3" => Value::int(optional_int.unwrap_or(0), span), "key4" => Value::bool(config.setting, span), } ``` Since macros hinder formatting, etc., the right hand side values should be relatively short and sweet like the examples above. Where possible, prefer `record!` or `.collect()` on an iterator instead of multiple `Record::push`s, since the first two automatically set the record capacity and do less work overall. # User-Facing Changes Besides the changes in `nu-protocol` the only other breaking changes are to `nu-table::{ExpandedTable::build_map, JustTable::kv_table}`.	2023-08-25 07:50:29 +12:00
Doru	dacf80f34a	Feature: Userland LazyRecords (#8332) ... # Description Despite the innocent-looking title, this PR involves quite a few backend changes as the existing LazyRecord trait was not at all friendly towards the idea of these values being generated on the fly from Nu code. In particular, here are a few changes involved: - The LazyRecord trait now involves a lifetime `'a`, and this lifetime is used in the return value of `get_column_names`. This means it no longer returns `'static str`s (but implementations still can return these). This is more stringent on the consumption side. - The LazyRecord trait now must be able to clone itself via a new `clone_value` method (as requiring `Clone` is not object safe). This pattern is borrowed from `Value::CustomValue`. - LazyRecord no longer requires being serde serializable and deserializable. These, in hand, allow for the following: - LazyRecord can now clone itself, which means that they don't have to be collected into a Record when being cloned. - This is especially useful in Stack, which is cloned on each repl line and in a few other cases. This would mean that _every_ LazyRecord instance stored in a variable would be collected in its entirety and cloned, which can be catastrophic for performance. See: `let nulol = $nu`. - LazyRecord's columns don't have to be static, they can have the same lifetime of the struct itself, so different instances of the same LazyRecord type can have different columns and values (like the new `NuLazyRecord`) - Serialization and deserialization are no longer meaningless, they are simply less. I would consider this PR very "drafty", but everything works. It probably requires some cleanup and testing, though, but I'd like some eyes and pointers first. # User-Facing Changes New command. New restrictions are largely internal. Maybe there are some plugins affected? Example of new command's usage: ``` lazy make --columns [a b c] --get-value { |name| print $"getting ($name)"; $name | str upcase } ``` You can also trivially implement something like `lazy make record` to take a record of closures and turn it into a getter-like lazy struct: ``` def "lazy make record" [ record: record ] { let columns = ($record | columns) lazy make --columns $columns --get-value { |col| do ($record | get $col) } } ``` Open to bikeshedding. `lazy make` is similar to `error make` which is also in the core commands. I didn't like `make lazy` since it sounded like some transformation was going on. # Tour for reviewers Take a look at LazyMake's examples. They have `None` as the results, as such they aren't _really_ correct and aren't being tested at all. I didn't do this because creating the Value::LazyRecord is a little tricky and didn't want to risk messing it up, especially as the necessary variables aren't available when creating the examples (like stack and engine state). Also take a look at NuLazyRecord's get_value implementation, or in general. It uses an Arc<Mutex<_>> for the stack, which must be accessed mutably for eval_block but get_value only provides us with a `&self`. This is a sad state of affairs, but I don't know if there's a better way. On the same code path, we also have pipeline handling, and any pipeline that isn't a Pipeline::Value will return Value::nothing. I believe returning a Value::Error is probably better, or maybe some other handling. Couldn't decide on which ShellError to settle with for that branch. The "unfortunate casualty" in the columns.rs file. I'm not sure just how bad that is, though, I simply had to fight a little with the borrow checker. A few leftover comments like derives, comments about the now non-existing serde requirements, and impls. I'll definitely get around to those eventually but they're in atm Should NuLazyRecord implement caching? I'm leaning heavily towards **yes**, this was one of the main reasons not to use a record of closures (besides convenience), but maybe it could be opt-out. I'd wonder about its implementation too, but a simple way would be to move a HashMap into the mutex state and keep cached values there.	2023-05-17 18:35:22 -05:00
Reilly Wood	3b5172a8fa	LazyRecord (#7619) ... This is an attempt to implement a new `Value::LazyRecord` variant for performance reasons. `LazyRecord` is like a regular `Record`, but it's possible to access individual columns without evaluating other columns. I've implemented `LazyRecord` for the special `$nu` variable; accessing `$nu` is relatively slow because of all the information in `scope`, and [`$nu` accounts for about 2/3 of Nu's startup time on Linux](https://github.com/nushell/nushell/issues/6677#issuecomment-1364618122). ### Benchmarks I ran some benchmarks on my desktop (Linux, 12900K) and the results are very pleasing. Nu's time to start up and run a command (`cargo build --release; hyperfine 'target/release/nu -c "echo \"Hello, world!\""' --shell=none --warmup 10`) goes from **8.8ms to 3.2ms, about 2.8x faster**. Tests are also much faster! Running `cargo nextest` (with our very slow `proptest` tests disabled) goes from **7.2s to 4.4s (1.6x faster)**, because most tests involve launching a new instance of Nu. ### Design (updated) I've added a new `LazyRecord` trait and added a `Value` variant wrapping those trait objects, much like `CustomValue`. `LazyRecord` implementations must implement these 2 functions: ```rust // All column names fn column_names(&self) -> Vec<&'static str>; // Get 1 specific column value fn get_column_value(&self, column: &str) -> Result<Value, ShellError>; ``` ### Serializability `Value` variants must implement `Serializable` and `Deserializable`, which poses some problems because I want to use unserializable things like `EngineState` in `LazyRecord`s. To work around this, I basically lie to the type system: 1. Add `#[typetag::serde(tag = "type")]` to `LazyRecord` to make it serializable 2. Any unserializable fields in `LazyRecord` implementations get marked with `#[serde(skip)]` 3. At the point where a `LazyRecord` normally would get serialized and sent to a plugin, I instead collect it into a regular `Value::Record` (which can be serialized)	2023-01-18 19:27:26 -08:00