Skip to content
This repository has been archived by the owner on Feb 4, 2025. It is now read-only.

Experiment with ways of getting data into JsonBuffer. #47

Closed

Conversation

davidmorgan
Copy link
Contributor

No description provided.

final mapKeys = List.generate(mapSize, (i) => i.toString());

void main() {
JsonBufferSerializeBenchmark0().report();
Copy link
Contributor

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?

Copy link
Contributor Author

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) {
Copy link
Contributor

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 :).

Copy link
Contributor Author

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);
Copy link
Contributor

@jakemac53 jakemac53 Aug 16, 2024

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.

Copy link
Contributor Author

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 :)

Copy link
Contributor Author

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.

Copy link
Contributor Author

@davidmorgan davidmorgan left a 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();
Copy link
Contributor Author

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) {
Copy link
Contributor Author

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);
Copy link
Contributor Author

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 :)

@davidmorgan
Copy link
Contributor Author

This is obsoleted by #51

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants