util: concatStringsSep(): Avoid quadratic string append

[nh2]

Repeated += makes no complexity guarantees; this does.

I also renamed the variables for some more clarity (to be aligned with the docs of the function).

[nh2]

I also noticed that the prim_concatStringSep is very optimistic, assuming that each list element is 32 chars long:

nix/src/libexpr/primops.cc

Lines 2113 to 2120 in e849b19

	string res;
	res.reserve((args[1]->listSize() + 32) * sep.size());
	bool first = true;
	for (unsigned int n = 0; n < args[1]->listSize(); ++n) {
	if (first) first = false; else res += sep;
	res += state.coerceToString(pos, *args[1]->listElems()[n], context);
	}

If a nix language user uses builtins.concatStringsSep on a long list of long strings (e.g. to construct some big file), this will also be O(n²).

Could we use the same approach there, computing state.coerceToString(pos, *args[1]->listElems()[n], context); for each element ahead of time, of alternatively res.reserve()ing the buffer size exponentially to avoid that from happening?

Edit: See PR #3942.

[edolstra]

Doesn't std::string grow the capacity by a constant factor when it exceeds the current capacity, thereby amortizing the cost to O(1) per character?

[nh2]

Doesn't std::string grow the capacity by a constant factor when it exceeds the current capacity, thereby amortizing the cost to O(1) per character?

Many implementations do, but the standard doesn't guarantee it, see full answer in #3942 (comment).

[stale]

I marked this as stale due to inactivity. → More info

[stale]

I closed this issue due to inactivity. → More info