phobos
phobos copied to clipboard
Introduce FixedAppender
This provides an appender that doesn't hold its data in an indirection which needs allocating, the lack of which reduces overhead substantially for situations where the amount of actual work done with the appender is small (e.g. the number of elements appended is small). For maybe 95% of use-cases, the user does not want to pass an appender around by value, in fact it's hard to see why that would be desirable, so we take advantage of that and create a non-copyable FixedAppender which can store all its data "in-place".
I intend to spread its use around Phobos initially, for testing the interface, then expose it publicly later (so we can defer arguing about the name too much for now plz).
A separate attempt was made to improve Appender by deferring the allocation to the first copy using copy-constructors, but this did not go well due to problems with inout, const etc. but also that had the consequence of causing an allocation in a somewhat unexpected - and different - place. Hence, it was abandoned for now in favour of this.
The following is a diff to std.json, which when applied with the test code below it & a big json file (taken from https://github.com/json-iterator/test-data/blob/master/large-file.json) led to a reduction in runtime from 400-430 ms to around 350 ± 8 ms
@@ -1133,7 +1133,7 @@ if (isSomeFiniteCharInputRange!T)
import std.uni : isSurrogateHi, isSurrogateLo;
import std.utf : encode, decode;
- auto str = appender!string();
+ auto str = fixedAppender!string();
Next:
switch (peekChar())
@@ -1301,7 +1301,7 @@ if (isSomeFiniteCharInputRange!T)
case '0': .. case '9':
case '-':
- auto number = appender!string();
+ auto number = fixedAppender!string();
bool isFloat, isNegative;
void readInteger()
@@ -1498,7 +1498,7 @@ Set the $(LREF JSONOptions.specialFloatLiterals) flag is set in `options` to enc
*/
string toJSON(const ref JSONValue root, in bool pretty = false, in JSONOptions options = JSONOptions.none) @safe
{
- auto json = appender!string();
+ auto json = fixedAppender!string();
toJSON(json, root, pretty, options);
return json.data;
}
test_json.d
import std.json;
import std.file;
import std.datetime.stopwatch : StopWatch;
import std.stdio : writeln;
void main() {
auto input = readText("large-file.json");
StopWatch sw;
sw.start;
auto output = input.parseJSON.toString;
sw.stop;
sw.peek.writeln;
write("output.json", output);
}
Thanks for your pull request and interest in making D better, @John-Colvin! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:andReturns:)
Please see CONTRIBUTING.md for more information.
If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + phobos#8789"
I think that test failure might be legit, looking in to it now.
Also considering a different approach using a prefix allocator... let's see...
@deadalnix suggests "inPlaceAppender" as a name, idk... better than "fixed".