[Cronet] Performance of NSURLSessionDataTask using Cronet is below platform for large response. |
||||
Issue descriptionIn some scenarios performance of NSURLSessionDataTask that uses Cronet stack is below platform. One of these scenarios is large size of response (>1mb). I've done some experiments with test http server running locally on the iPad Mini2 (MF544LL/A) (https://codereview.chromium.org/2776583004) and got the following numbers for 10mb downloads: Platform: Elapsed Average:93.586791 ms Max:112.994969 ms Cronet: Elapsed Average:141.780800 ms Max:173.210979 ms This issue will be used to track the work.
,
Mar 29 2017
droger@ - by any chance do you remember why crn_http_protocol_handler.mm uses 4k read buffer? I've tried to increase it to 32k - 128k and got promising results (in synthetic test).
,
Mar 29 2017
"Platform: Elapsed Average:93.586791 ms Max:112.994969 ms Cronet: Elapsed Average:141.780800 ms Max:173.210979 ms" @Misha, Is this happening after we disable the HTTP cache? seems so
,
Apr 4 2017
,
Apr 11 2017
,
Apr 13 2017
Ping, I've run same test with different cache configurations, and disk cache affects performance of this particular test very negatively, especially when buffer size is 4k.
,
Apr 13 2017
by disabling cache, Cronet will be better in this particular test case? do we still need to increase the buffer size to see the gain? if yes, what's a good buffer size to use?
,
Apr 14 2017
Yes, by disabling cache I see Cronet getting better results in my test. Increasing buffer size from 4kb to 64-128kb shows best results in this test.
,
Apr 14 2017
I think 4k was used because that was what desktop was using at that time. It seemed to work well, and we definitely didn't try to tweak the value.
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e commit 054d4e484b45c4c7872eec3459ccb1c5f85a3b1e Author: mef <mef@chromium.org> Date: Wed Apr 19 20:41:19 2017 Use 64kb instead of 4kb read buffer in CRNHttpProtocolHandler on iOS. This reduces number of reads and reallocations if UrlRequest::Read has more data available. Change QuicTestServer to return simple response without HTTP/2 trailers, so it doesn't break the QuicHttpStream. BUG= 706515 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:ios-simulator-cronet Review-Url: https://codereview.chromium.org/2776583004 Cr-Commit-Position: refs/heads/master@{#465738} [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/components/cronet/ios/test/cronet_http_test.mm [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/components/cronet/ios/test/start_cronet.mm [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/components/cronet/ios/test/test_server.cc [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/components/cronet/ios/test/test_server.h [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/components/grpc_support/test/quic_test_server.cc [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/components/grpc_support/test/quic_test_server.h [modify] https://crrev.com/054d4e484b45c4c7872eec3459ccb1c5f85a3b1e/ios/net/crn_http_protocol_handler.mm
,
May 10 2017
Misha, do you android cronet url request can benefit for a larger buffer size as well? Right now we are using the default buffer size of 32KB, same as the one in the android example. private static final int READ_BUFFER_SIZE = 32 * 1024; mBuffer = ByteBuffer.allocateDirect(READ_BUFFER_SIZE);
,
May 10 2017
Zhihua, that's a great question. I would expect that it depends on network, device and app usage. Cronet UrlRequest on Android lets app to provide its own buffer, and you may want to experiment to see if bigger or smaller buffer is better.
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5872bbebe979ebb59886bfc20f91683ff313a594 commit 5872bbebe979ebb59886bfc20f91683ff313a594 Author: Lily Houghton <lilyhoughton@chromium.org> Date: Tue Oct 31 14:56:59 2017 [Cronet] Basic performance testing. This adds some very basic performance testing to the Cronet test suite. Bug: 706515 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I64f4a24baf8fe92d2d9d865752cbff4bcb2df12c Reviewed-on: https://chromium-review.googlesource.com/649991 Reviewed-by: Misha Efimov <mef@chromium.org> Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org> Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Cr-Commit-Position: refs/heads/master@{#512830} [modify] https://crrev.com/5872bbebe979ebb59886bfc20f91683ff313a594/components/cronet/ios/test/BUILD.gn [modify] https://crrev.com/5872bbebe979ebb59886bfc20f91683ff313a594/components/cronet/ios/test/cronet_http_test.mm [add] https://crrev.com/5872bbebe979ebb59886bfc20f91683ff313a594/components/cronet/ios/test/cronet_performance_test.mm [modify] https://crrev.com/5872bbebe979ebb59886bfc20f91683ff313a594/components/cronet/ios/test/cronet_test_base.h [modify] https://crrev.com/5872bbebe979ebb59886bfc20f91683ff313a594/components/cronet/ios/test/cronet_test_base.mm
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0c815cfa7f855fb4494def7dfe7e1dec97941f9c commit 0c815cfa7f855fb4494def7dfe7e1dec97941f9c Author: kapishnikov <kapishnikov@chromium.org> Date: Thu Nov 30 18:01:01 2017 Use "<=" instead of "<" in perf test It is not entirely correct to use "<" to compare number of failed requests. E.g. it will always fail if the total number of requests is '1'. If the total number of requests is 3 then no failures are tolerated while one should be allowed. BUG= 706515 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I77469141d5b789ca793a1cf8c3a79a9de5206a84 Reviewed-on: https://chromium-review.googlesource.com/800973 Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#520598} [modify] https://crrev.com/0c815cfa7f855fb4494def7dfe7e1dec97941f9c/components/cronet/ios/test/cronet_performance_test.mm
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7ac949be778b9726fe2e3c5cdbcd31f64586b7c commit d7ac949be778b9726fe2e3c5cdbcd31f64586b7c Author: kapishnikov <kapishnikov@chromium.org> Date: Thu Nov 30 18:08:57 2017 Fix printing of downloaded bytes in Cronet perf tests. [delegate_ totalBytesReceivedPerTask][task] returns a NSNumber that should be converted to integer for proper printing. BUG= 706515 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I57c7ecea727a13ef5a79a205dee1c134dfe93eea Reviewed-on: https://chromium-review.googlesource.com/797952 Reviewed-by: Misha Efimov <mef@chromium.org> Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org> Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Cr-Commit-Position: refs/heads/master@{#520600} [modify] https://crrev.com/d7ac949be778b9726fe2e3c5cdbcd31f64586b7c/components/cronet/ios/test/cronet_performance_test.mm
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7fe493a1b29becbc069b6a7b8e58cf51b23b27d8 commit 7fe493a1b29becbc069b6a7b8e58cf51b23b27d8 Author: kapishnikov <kapishnikov@chromium.org> Date: Thu Nov 30 18:23:54 2017 Use Int64_t for PerfResult.total_bytes_downloaded to avoid overflow. BUG= 706515 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I4e5a3fb4b42c8367abb0e46d8f5383950442e064 Reviewed-on: https://chromium-review.googlesource.com/801050 Reviewed-by: Misha Efimov <mef@chromium.org> Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org> Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Cr-Commit-Position: refs/heads/master@{#520609} [modify] https://crrev.com/7fe493a1b29becbc069b6a7b8e58cf51b23b27d8/components/cronet/ios/test/cronet_performance_test.mm
,
Jan 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d5eb3c9d43994e50643107e90733fe36afeb1d8 commit 6d5eb3c9d43994e50643107e90733fe36afeb1d8 Author: kapishnikov <kapishnikov@chromium.org> Date: Thu Jan 18 17:58:33 2018 Fix the iOS performance test to correctly detect failed requests Without the change the test considers a requests failed only if it timed out. It doesn't take into consideration the response staus code. BUG= 706515 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ibe938c0486547cbe64ce2545fa016b71c28ff1d3 Reviewed-on: https://chromium-review.googlesource.com/862665 Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Reviewed-by: Misha Efimov <mef@chromium.org> Cr-Commit-Position: refs/heads/master@{#530195} [modify] https://crrev.com/6d5eb3c9d43994e50643107e90733fe36afeb1d8/components/cronet/ios/test/cronet_performance_test.mm [modify] https://crrev.com/6d5eb3c9d43994e50643107e90733fe36afeb1d8/components/cronet/ios/test/cronet_pkp_test.mm [modify] https://crrev.com/6d5eb3c9d43994e50643107e90733fe36afeb1d8/components/cronet/ios/test/cronet_test_base.h [modify] https://crrev.com/6d5eb3c9d43994e50643107e90733fe36afeb1d8/components/cronet/ios/test/cronet_test_base.mm
,
Jan 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d9af179d07ce51b2facc781fdeabc5e7b6863f7 commit 9d9af179d07ce51b2facc781fdeabc5e7b6863f7 Author: kapishnikov <kapishnikov@chromium.org> Date: Thu Jan 18 18:05:15 2018 Fixed a bug in throughput calculation of iOS perf tests That was the reason why throughput on iOS was shown 10x lower than on Android. BUG= 706515 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: If8e583c547e3f196431279e3ceb29efd113971e8 Reviewed-on: https://chromium-review.googlesource.com/864742 Reviewed-by: Misha Efimov <mef@chromium.org> Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org> Cr-Commit-Position: refs/heads/master@{#530200} [modify] https://crrev.com/9d9af179d07ce51b2facc781fdeabc5e7b6863f7/components/cronet/ios/test/cronet_performance_test.mm
,
Jan 3
Performance is now on par. |
||||
►
Sign in to add a comment |
||||
Comment 1 by mef@chromium.org
, Mar 29 2017Owner: mef@chromium.org
Status: Assigned (was: Untriaged)