> -/*
> - * Send text representation of one attribute, with conversion and escaping
> - */
> #define DUMPSOFAR() \
IIUC this comment was meant to describe the CopyAttributeOutText() function
just below this macro. When the macro was added in commit 0a5fdb0 from
2006, the comment became detached from the function. Maybe we should just
move it back down below the macro.
Ah, that makes sense -- done.
> +/*
> + * Send text representation of one attribute, with conversion and CSV-style
> + * escaping. This variant uses SIMD instructions to optimize processing, but
> + * we can only use this approach when encoding_embeds_ascii if false.
> + */
nitpick: Can we add a few words about why using SIMD instructions when
encoding_embeds_ascii is true is difficult? I don't dispute that it is
complex and/or not worth the effort, but it's not clear to me why that's
the case just from reading the patch.
Sounds good.
> +static void
> +CopyAttributeOutCSVFast(CopyToState cstate, const char *ptr,
> + bool use_quote)
nitpick: Can we add "vector" or "simd" to the name instead of "fast"? IMHO
it's better to be more descriptive.
Sure, done.
Attached is a revised patch series, that incorporates the feedback above and makes two additional changes:
* Add some regression tests to cover COPY behavior with octal and hex escape sequences
* Optimize the COPY TO text (non-CSV) code path (CopyAttributeOutText()).
In CopyAttributeOutText(), I refactored some code into a helper function to reduce code duplication, on the theory that field delimiters and escape sequences are rare, so we don't mind taking a function call in those cases.
We could go further and use the same code to handle both the tail of the string in the vectorized case and the entire string in the non-vectorized case, but I didn't bother with that -- as written, it would require taking an unnecessary strlen() of the input string in the non-vectorized case.
Performance for COPY TO in text (non-CSV) mode:
===
master
Benchmark 1: ./psql -f /Users/neilconway/copy-out-bench-text-long-strings.sql
Time (mean ± σ): 1.240 s ± 0.013 s [User: 0.001 s, System: 0.000 s]
Range (min … max): 1.220 s … 1.256 s 10 runs
Benchmark 1: ./psql -f /Users/neilconway/copy-out-bench-text-short.sql
Time (mean ± σ): 522.3 ms ± 11.3 ms [User: 1.2 ms, System: 0.0 ms]
Range (min … max): 512.0 ms … 544.3 ms 10 runs
master + SIMD patches:
Benchmark 1: ./psql -f /Users/neilconway/copy-out-bench-text-long-strings.sql
Time (mean ± σ): 867.6 ms ± 12.7 ms [User: 1.2 ms, System: 0.0 ms]
Range (min … max): 842.1 ms … 891.6 ms 10 runs
Benchmark 1: ./psql -f /Users/neilconway/copy-out-bench-text-short.sql
Time (mean ± σ): 536.7 ms ± 10.9 ms [User: 1.2 ms, System: 0.0 ms]
Range (min … max): 530.1 ms … 566.8 ms 10 runs
===
Looks like there is a slight regression for short attribute values, but I think the tradeoff is a net win.
I'm going to take a look at applying similar ideas to COPY FROM next.
Neil
