-
Notifications
You must be signed in to change notification settings - Fork 7
Experiment with ways of getting data into JsonBuffer. #47
Experiment with ways of getting data into JsonBuffer. #47
Conversation
final mapKeys = List.generate(mapSize, (i) => i.toString()); | ||
|
||
void main() { | ||
JsonBufferSerializeBenchmark0().report(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fwiw calling each benchmark 3 times shouldn't be necessary... it does its own warmup. It is possible the warmup isn't running enough times or something? We can override how warmup works if we need which might be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it to get an idea of the variance, then noticed that indeed the first run of the first benchmark was slower. Looking closer the runs do not seem to be particularly stable, sometimes there is a first benchmark difference, sometimes not, and there are quite large differences between runs ... I should really be running AOT compiled anyway, I guess, will switch to doing that.
@@ -25,7 +93,75 @@ class JsonBufferSerializeBenchmark extends BenchmarkBase { | |||
|
|||
@override | |||
void run() { | |||
serialized = JsonBuffer(LazyMap(mapKeys, (key) => key)).serialize(); | |||
serialized = JsonBuffer(LazyMap(mapKeys, (key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a single canonical mapKeys
list like this is cheating some, I don't think we will ever be able to use pre-existing lists for keys in practice because we are always providing a filtered view on objects (except maybe if somebody asks for every field we could special case that).
Unless we always serialize null values too, but if we were going to do that I would probably advocate for an entirely different approach to serialization :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, will see if this can be a bit more realistic.
|
||
extension type InterfaceBuilder._(JsonBuffer buffer) { | ||
factory InterfaceBuilder() { | ||
runningBuffer!.startMap(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we were going to do an approach like this, I think we would want something like a JsonBufferView
which could encapsulate the logic of the reserved space for the map, with the ability to allocate new objects in the original buffer and overwrite the original pointer values to point at those, so that we could hopefully remove this entire notion of runningBuffer
.
Yes, it would mean additional allocations but this approach seems too brittle to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will experiment :)
I actually like a lot the idea of having access to the JsonBuffer
that is where we are building up the response to the current request. The idea of "request scope" is heavily used in Guice, it's natural in server or client code to have exactly one of something per request. In Dart I guess we can do that nicely with zones. We can always pass around context explicitly if that turns out to be too brittle.
Separate to that, it occurred to me that we can probably gain a lot by being upfront about the type information that we have.
So for example for Properties
we know the set of keys and we know the values are all bools.
If we tell JsonBuffer
what we're doing, it should be possible to write the keys just once, and to pack the bools into bit each. Which will be both smaller and faster :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, yes, definitely a split to a type for writing and a type for reading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Will see if I can turn this into something actionable without spending too much time on it ;)
final mapKeys = List.generate(mapSize, (i) => i.toString()); | ||
|
||
void main() { | ||
JsonBufferSerializeBenchmark0().report(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it to get an idea of the variance, then noticed that indeed the first run of the first benchmark was slower. Looking closer the runs do not seem to be particularly stable, sometimes there is a first benchmark difference, sometimes not, and there are quite large differences between runs ... I should really be running AOT compiled anyway, I guess, will switch to doing that.
@@ -25,7 +93,75 @@ class JsonBufferSerializeBenchmark extends BenchmarkBase { | |||
|
|||
@override | |||
void run() { | |||
serialized = JsonBuffer(LazyMap(mapKeys, (key) => key)).serialize(); | |||
serialized = JsonBuffer(LazyMap(mapKeys, (key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, will see if this can be a bit more realistic.
|
||
extension type InterfaceBuilder._(JsonBuffer buffer) { | ||
factory InterfaceBuilder() { | ||
runningBuffer!.startMap(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will experiment :)
I actually like a lot the idea of having access to the JsonBuffer
that is where we are building up the response to the current request. The idea of "request scope" is heavily used in Guice, it's natural in server or client code to have exactly one of something per request. In Dart I guess we can do that nicely with zones. We can always pass around context explicitly if that turns out to be too brittle.
Separate to that, it occurred to me that we can probably gain a lot by being upfront about the type information that we have.
So for example for Properties
we know the set of keys and we know the values are all bools.
If we tell JsonBuffer
what we're doing, it should be possible to write the keys just once, and to pack the bools into bit each. Which will be both smaller and faster :)
This is obsoleted by #51 |
No description provided.