Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
table.concat is idiomatic and should be the fastest way to concatenate all table array elements together, but apparently you can beat it by using
string.format
,string.rep
andtable.unpack
:... this just won't do, so we should fix table.concat performance.
The deficit comes from two places:
This change fixes this by using a fast path for in-bounds array lookup for a string. Note that
table.concat
also supports numbers (these need to be converted to strings which is a little cumbersome and has innate overhead), and out-of-bounds accesses*. In these cases we fall back to the old implementation.To trigger out-of-bounds accesses, you need to skip the past-array-end element (which is nil per array invariant), but this is achievable because table.concat supports offset+length arguments. This should almost never come up in practice but the per-element branches et al are fairly cheap compared to the eventual string copy/alloc anyway.
This change makes table.concat ~2x faster when the separator is empty; the table.concat benchmark shows +40% gains but it uses a variety of string separators of different lengths so it doesn't get the full benefit from this change.