Conversation
This directly streams strings from _tojson_stream and avoids string concatenation and flattening arrays, to lower memory usage. Furthermore, the ANSI color, stream separator, and indentation constructions are moved earlier.
Now, color and output options affect the formatting of values and --no-builtins affects the environment in the REPL.
53746ce to
d6d9e4e
Compare
wader
left a comment
There was a problem hiding this comment.
Great stuff! thanks
I wonder if it's soon time for some tojson tests
| end | ||
| | if $opts.raw_no_lf | not then ., "\n" end | ||
| | if $opts.raw_output0 then ., "\u0000" end | ||
| | dump($opts) |
There was a problem hiding this comment.
It's just like how it was before color support! Just with dump($opts) instead of tojson haha
|
Yeah, it could probably use some of those. jq's tests which cover colors are written in bash, so it would entail writing more bash 😈. It might be possible in jq even without the Good thing you mentioned it, though. I ran my local test from when I was fixing reset consistency with jq colors, and found a bug, which has now been patched. |
|
I wonder if we could extend the test format somehow to make it nicer new lines in the output and maybe escaped hex etc? maybe some kind of "raw" string mode for outputs? |
|
👍 i try to add tests even for the most trivial things in my projects, i just don't trust myself :) |
|
Or there could be a flag to |
Could you give an example? My aim is mostly to not end up with tests where the expected output is one-liner messes, i'm thinking something: |
|
I think multiline strings would solve part of it (though since the closing delimiter starts a line, we'd need to ignore the LF before it, so we could differentiate text with a trailing LF and without). The bigger problem I see is that we have no way to configure the CLI options passed. Perhaps a directive like |
Yeap!
That would be great. for fq i made the test format be more or less a "transcript" so that tests looks exactly like normal shell invocations, ex https://github.com/wader/fq/blob/master/pkg/interp/testdata/debug.fqtest, that has worked very well. Also makes it quite easy to automatically rewrite expected with new actual output. (Just don't look at the implementation, is a less) |
|
I really like the format in your fq tests. Using $ as the prefix doubles as invalid jq syntax, so it couldn’t be confused. I’d only change it to add an LF between tests. Is the hacky part of it the quote handling? I was annoyed recently with fromjson not interacting nicely with single quotes. Maybe it could be done by replacing " with something from the Unicode private use area, then ' with ", passing it to fromjson, then restoring ". |
Yes might be good idea, it can feel a bit cramped now
The thing that caused a bit of problem in the beginning was how to split the different tests when you only the the prompt to split on, is especially with the REPL support. At the moment the transcript parser is mess of balanced regex :) https://github.com/wader/fq/blob/master/internal/script/script.go#L467 :shame: |
I have refactored
_tojsonto stream strings and reuse concatenations. It is now significantly faster, and has fully recovered from the performance regression when I added colors in #14.Since jq is buffered by default, I saw no penalties from streaming tiny strings instead of concatenating into larger chunks like lines. There may be a sweet spot for chunk sizes that would suit
jq --unbufferedand gojq, but I didn't benchmark it carefully like the rest.Benchmarks
The
.benchmark exercises the streaming behavior and thetojsonbenchmark collects it into an array and joins it. Notice thatstreamed(tojson) is now on faster thanbefore-colors(tojson), so that performance loss has been fully regained.before-colors(.) is actually deceptive, because that revision dumped using hosttojsoninstead of_tojson. There's no good comparison for., but it seems to be as expected, matchingtojson.streamed: d6d9e4e (Fold newline into indent prefix, 2024-03-08), i.e., this PRmaster: d0fdc43 (Make eval work in eval, 2024-03-08), i.e.,origin/masterat the time of this PRmaster-passthrough: d0fdc43 (Make eval work in eval, 2024-03-08) with_tojsonstubbed with hosttojsonafter-colors: 0532d9c (Fix --help with --join-output, 2024-02-07), i.e., the final commit related to color supportbefore-colors: 8a02423 (Add query? shorthand for try query catch empty, 2024-01-30), i.e., the commit before I added color supportstreamed/jqjq . < large.jsonstreamed/jqjq tojson < large.jsonmaster/jqjq . < large.jsonmaster/jqjq tojson < large.jsonmaster-passthrough/jqjq . < large.jsonmaster-passthrough/jqjq tojson < large.jsonafter-colors/jqjq . < large.jsonafter-colors/jqjq tojson < large.jsonbefore-colors/jqjq . < large.jsonbefore-colors/jqjq tojson < large.jsonGenerated with: