batchingexperimental: add RequestAccumulator

[pongad]

Instead of exposing one surface that knows how to batch end-to-end,
this PR explores ways to expose multiple surfaces
to be mixed and matched by individual clients.

RequestAccumulator accumulates requests into batches, but leaves
RPC calls, error handling, etc to the caller.

[pongad]

Submitting to a branch, but we should actually measure accumulator's performance before going too far.

@igorbernstein2 I think this ought to fit pubsub and logging's use case. Do you think it fits bigtable's?

[igorbernstein2]

I like the simplicity of this and it reminds me of the go implementation of bundling. Bigtable client will also need to deal with concurrent batches and flow control. Is that in scope for another shared component?

[garrettjonesgoogle]

I'd like to see the impact to Logging too

[pongad]

@igorbernstein2 PTAL.

Bigtable client will also need to deal with concurrent batches and flow control. Is that in scope for another shared component?

For concurrent batch, do you mean sending second batch before the first returns? Pubsub is already doing this. You can substitute the // ... in the class doc with an async call. You'll need to copy lists that you want to retain though.

For flow control, I think it's definitely in scope. Pubsub team requested us to not put it into pubsub client yet, and I'd like to see two use-cases before I start abstracting. My current opinion: You and I can write flow control ourselves for Logging and BigTable first. Then we can see what they have in common (could be "everything"), then we can refactor. It's more work, but I think it will help with design details. WDYT?

[igorbernstein2]

For concurrent batch, do you mean sending second batch before the first returns?

Yes.

You'll need to copy lists that you want to retain though.

I think we can efficiently avoid this by storing the extra item in a separate variable. Then batch() can blindly return the list of elements and next() can always create a new list instead of truncating the existing list (inserting the extra item if needed)

I think this sounds fine. Only that's missing here is that curBytes needs to be exposed so that flow control can use the same numbers as the accumulator.

[pongad]

I'd like to see the impact to Logging too

@garrettjonesgoogle I'd like to work on this in a separate PR, since this one is already getting large. We're in a branch, so editing things should be easy.

I think we can efficiently avoid this by storing the extra item in a separate variable.

@igorbernstein2 I think this isn't quite optimal. When working with protobuf, you have to do something like request.addAllItems(accumulator.batch()). addAll copies, so we essentially get the copy for "free" in the sense that we need to do that anyway. I'm sort of hoping that clear() doesn't downsize the backing array, so that we don't need to allocate temporary lists to move items.

Only that's missing here is that curBytes needs to be exposed

I agree but I'd like to do this in a separate PR. It deserves some tests.

Unless either of you fundamentally disagree with the premise of this PR. I think we should merge this.
Things like naming and function signatures can be hashed out a little later. We're in a branch anyway. WDYT?

[igorbernstein2]

SGTM

[garrettjonesgoogle]

In general, I won't block any merges to the branch.