[cronet] Pass total bytes received to CronetUrlRequest.onSucceeded()

[cronet] Pass total bytes received to CronetUrlRequest.onSucceeded()

Previously onSucceeded() had to synchronize and turn around and
request total bytes received.  By passing it we avoid the
synchronization.

Committed: https://crrev.com/255fd2c4d20fb71063f9ed244c17df5063387ed2
Cr-Commit-Position: refs/heads/master@{#327334}

[pauljensen, 2015-04-22 19:09:28]

pauljensen@chromium.org changed reviewers:
+ mmenke@chromium.org

[pauljensen, 2015-04-22 19:09:29]

Matt, PTAL.  Not really a significant performance win but it's less code. :/

[mmenke, 2015-04-22 19:13:22]

I'm all in favor of reducing the number of Java->C++ (And C++->Java) calls, even
if they don't cost us much performance.

https://codereview.chromium.org/1100923002/diff/1/components/cronet/android/j...
File components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
(right):

https://codereview.chromium.org/1100923002/diff/1/components/cronet/android/j...
components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java:710:
private native long nativeGetTotalReceivedBytes(long nativePtr);
I don't think we need this any more (And the corresponding C++ code, of course)

[pauljensen, 2015-04-24 11:18:16]

I think a JNI call costs roughly one microsecond on say a Nexus 5, but a
synchronized block can cost 100-3000 microseconds with no contention.

https://codereview.chromium.org/1100923002/diff/1/components/cronet/android/j...
File components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
(right):

https://codereview.chromium.org/1100923002/diff/1/components/cronet/android/j...
components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java:710:
private native long nativeGetTotalReceivedBytes(long nativePtr);
On 2015/04/22 19:13:22, mmenke (Out 4-27 to 5-1) wrote:
> I don't think we need this any more (And the corresponding C++ code, of
course)

Done.

[pauljensen, 2015-04-24 11:42:41]

On 2015/04/24 11:18:16, pauljensen wrote:
> I think a JNI call costs roughly one microsecond on say a Nexus 5, but a
> synchronized block can cost 100-3000 microseconds with no contention.
On second thought my estimate of the synchronized block includes an
Object.notify() call.

[mmenke, 2015-04-24 11:59:31]

Looks like Java does have atomics.

On Fri, Apr 24, 2015 at 7:42 AM, <pauljensen@chromium.org> wrote:

> On 2015/04/24 11:18:16, pauljensen wrote:
>
>> I think a JNI call costs roughly one microsecond on say a Nexus 5, but a
>> synchronized block can cost 100-3000 microseconds with no contention.
>>
> On second thought my estimate of the synchronized block includes an
> Object.notify() call.
>
> https://codereview.chromium.org/1100923002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[pauljensen, 2015-04-27 17:29:07]

pauljensen@chromium.org changed reviewers:
+ mef@chromium.org

[pauljensen, 2015-04-27 17:29:07]

Adding Misha to review while Matt is out.

[mef, 2015-04-28 16:07:18]

lgtm

[pauljensen, 2015-04-28 17:50:35]

The CQ bit was checked by pauljensen@chromium.org

[commit-bot: I haz the power, 2015-04-28 17:50:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1100923002/40001

[commit-bot: I haz the power, 2015-04-28 18:26:46]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-28 18:28:19]

Patchset 3 (id:??) landed as
https://crrev.com/255fd2c4d20fb71063f9ed244c17df5063387ed2
Cr-Commit-Position: refs/heads/master@{#327334}