Buffer pooling for JSON serialization

[Scooletz]

Addresses #640

This PR Introduces CharArrayTextWriter that by provides access to its char[]. Additionally changes the way that driver parameters are created, enables reusing memory and reducing allocations for serializing.

Plan

• change DocJsonBodyArgument to use pooled writer
• change EventStreamAppender to use pooled writers
• test buffering
• decide what's the best buffering strategy:
  • one pool per app (current approach)
  • one per DocumentStore
  • additional pool for session to keep leased writers for reuse during the same session and return at the end of the session
  • another one?
• decide should this be opt-in or opt-out behavior

Notes:

• The EventStreamAppender can't be changed now as Npgsql does not support writing ArraySegment<char> as jsonb, json nor string. I took a look at the implementation of the driver at it would require a lot of work as the WriteBuffer does not have overloads for this. If we had it, then appending events would be easy.

[Scooletz]

@jeremydmiller Can we change the seam? From semver perspective this is going to be a breaking change (major). We could provide a new interface that would be implemented by serializers. For now I assume it's ok to change this.

[jeremydmiller]

It's not a major change if it's additive. I say yes.

[Scooletz]

It broke DocJsonBodyArgument as it was selecting the method by name ;-)

[jeffdoolittle]

Should the default be to use char buffer pooling? I'm not a fan of having a bool default to true so perhaps consider flipping the name and logic around to something like SuppressCharBufferPooling.

[Scooletz]

In majority of cases I prefer to use opt-in, then opt-out in a next version of a product. I'm not changing the behavior now as it's easier to test with no caching and focus on a few tests with caching. I'll add this as the point to this PR though.

[jeffdoolittle]

That sounds like a good plan

[Scooletz]

@jeremydmiller The initial phase of implementation is done in let's say 60%. I use only one test for testing the augmented UpdateBatch behavior. Could you give me some feedback on the approach I took? What's the convention for tests as well? I used only one Fact with a few assertions, which might be not good enough.

Also, I've just found Resolver<T>.RegisterUpdate method accepting json as a parameter. It looks that it isn't used at all. Additionally, the overload without JSON serializers the entity. This means that it's not needed. Am I missing something or is it useless?

[Scooletz]

I followed the way Marten handles dirty checking. This lead me to fetching documents which again uses strings. If we wanted to make no allocations that would require to change the fetch mechanism as well and use the char buffers in there.

☝️ This is a some kind of note to myself. I'll try to play a little bit with it and post my findings in here.

[Scooletz]

Ok, I went through the API again. Basically, IResolver<T> and other parts (especially dirty checking) would require to replace string json with the new char writer. We could even read from the reader using GetChars instead of the current GetString.

I'm looking forward to hearing from you as it requires a lot of effort @jeremydmiller

[jeremydmiller]

@Scooletz I'm pulling this into master as much as anything to make rebase's not be a problem. The one and only change I made was to turn on character pooling by default. That exposed a single test failure, but that one was strictly a testing problem.

@Scooletz

I say it looks good as is. The next thing I'd love to do is to add some profiling w/ Benchmark.Net to check the allocations with and without the pooling turned on. After that, I'd like to retrofit some tests that hit this w/ multiple threads pretty hard.

Correct me if I'm wrong, but this is only applied to writing documents in the DocumentSession.SaveChanges() / UpdateBatch mechanics, right? Doesn't look like it applies to bulk inserts yet, and we still need to do something completely different for reads?

[Scooletz]

👍 for Benchmarks.Net. This could be a separate PR on it's own.

Yes, this is only applied to writing documents in the DocumentSession.SaveChanges() / UpdateBatch mechanics. Yes and yes :) For reads, this will involve replacing string json with something else. For events, I'd need to send a PR to npgsql to allow ArraySegment<char> in the following lines: https://github.com/npgsql/npgsql/blob/4ef74fa78cffbb4b1fdac00601d0ee5bff5e242b/src/Npgsql/TypeHandlers/TextHandler.cs#L269-L276