Create unbuffered Tumble entry point for LogDog.

Create unbuffered Tumble entry point for LogDog.

Add RegisterInTransaction, an entry point for Tumble that initiates a
standard datastore transaction, performs an operation, and registers the
ensuing mutation against it. This is different from RunMutation in that
it doesn't require a Mutation as a starting point and also doesn't
create a transaction buffer.

Leverage this in LogDog's "registerStream" to reduce the overhead of
stream registration.

BUG=chromium:675485
TEST=unit

Review-Url: https://codereview.chromium.org/2592753002
Committed: https://github.com/luci/luci-go/commit/464d9ab6bc1614432afa076e4416b0033028cd7b

[dnj, 2016-12-20 18:28:27]

dnj@chromium.org changed reviewers:
+ iannucci@chromium.org, vadimsh@chromium.org

[dnj, 2016-12-20 18:28:28]

PTAL. This is a smaller optimization, but I think it accomplishes two things:
1) It makes it so that we don't have to pay for an "impl/memory" instance each
time.
2) It clearly functions as an entry point, instead of the somewhat-obscure
"RunMutation", which requires an actual Mutation instance to do otherwise-inline
operations.

WDYT?

[Vadim Sh., 2016-12-20 21:25:15]

lgtm

https://codereview.chromium.org/2592753002/diff/1/logdog/appengine/coordinato...
File logdog/appengine/coordinator/service.go (right):

https://codereview.chromium.org/2592753002/diff/1/logdog/appengine/coordinato...
logdog/appengine/coordinator/service.go:260: // Use an AppEngine Context so we
have access to the socket API. This is
this doesn't look like a part of this CL

[iannucci, 2016-12-21 10:05:19]

can we please quantify the performance improvement? Is this 1% faster? 20%
faster? 100x faster?

https://codereview.chromium.org/2592753002/diff/20001/tumble/tumble.go
File tumble/tumble.go (right):

https://codereview.chromium.org/2592753002/diff/20001/tumble/tumble.go#newcode35
tumble/tumble.go:35: // RegisterInTransaction opens a lightweight unbuffered
transaction on "root"
Why not 'RunUnbuffered'? There's no registration here, and this is functionally
equivalent to RunMutation.

https://codereview.chromium.org/2592753002/diff/20001/tumble/tumble.go#newcode40
tumble/tumble.go:40: // overhead of spinning up a new buffered transaction.
s/all of the//

[dnj, 2016-12-21 19:12:18]

On 2016/12/21 10:05:19, iannucci wrote:
> can we please quantify the performance improvement? Is this 1% faster? 20%
> faster? 100x faster?
> 
> https://codereview.chromium.org/2592753002/diff/20001/tumble/tumble.go
> File tumble/tumble.go (right):
> 
>
https://codereview.chromium.org/2592753002/diff/20001/tumble/tumble.go#newcode35
> tumble/tumble.go:35: // RegisterInTransaction opens a lightweight unbuffered
> transaction on "root"
> Why not 'RunUnbuffered'? There's no registration here, and this is
functionally
> equivalent to RunMutation.
> 
>
https://codereview.chromium.org/2592753002/diff/20001/tumble/tumble.go#newcode40
> tumble/tumble.go:40: // overhead of spinning up a new buffered transaction.
> s/all of the//

This resulted form heap analysis of running services. I observed that the
majority of allocations and allocation paths included "impl/memory" via txnBuf.
Since those are not useful at all here, it seemed prudent to remove it.

I added a benchmark. It's not a big difference either way, but the actual
disparity goes up the more Put operations we run within the transaction, which
is obvious. The real "win" to this method is that we aren't paying memory cost
twice for each operation.

[iannucci, 2016-12-22 01:18:56]

Oops! I meant to lgtm this the other day.

Thanks for adding a benchmark.

[dnj, 2016-12-22 01:36:49]

The CQ bit was checked by dnj@chromium.org

[dnj, 2016-12-22 01:36:49]

The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org
Link to the patchset: https://codereview.chromium.org/2592753002/#ps40001
(title: "Add bench, update.")

[commit-bot: I haz the power, 2016-12-22 01:36:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-22 01:43:30]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482370609256400,
"parent_rev": "8c47ef91aef491e61dd8be748442012d48763350", "commit_rev":
"464d9ab6bc1614432afa076e4416b0033028cd7b"}

[commit-bot: I haz the power, 2016-12-22 01:43:33]

Description was changed from

==========
Create unbuffered Tumble entry point for LogDog.

Add RegisterInTransaction, an entry point for Tumble that initiates a
standard datastore transaction, performs an operation, and registers the
ensuing mutation against it. This is different from RunMutation in that
it doesn't require a Mutation as a starting point and also doesn't
create a transaction buffer.

Leverage this in LogDog's "registerStream" to reduce the overhead of
stream registration.

BUG=chromium:675485
TEST=unit
==========

to

==========
Create unbuffered Tumble entry point for LogDog.

Add RegisterInTransaction, an entry point for Tumble that initiates a
standard datastore transaction, performs an operation, and registers the
ensuing mutation against it. This is different from RunMutation in that
it doesn't require a Mutation as a starting point and also doesn't
create a transaction buffer.

Leverage this in LogDog's "registerStream" to reduce the overhead of
stream registration.

BUG=chromium:675485
TEST=unit

Review-Url: https://codereview.chromium.org/2592753002
Committed:
https://github.com/luci/luci-go/commit/464d9ab6bc1614432afa076e4416b0033028cd7b
==========

[commit-bot: I haz the power, 2016-12-22 01:43:33]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-go/commit/464d9ab6bc1614432afa076e4416b0033028cd7b