New issue
Advanced search Search tips

Issue 869406 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS
Pri: 3
Type: Bug

Blocking:
issue 786559



Sign in to add a comment

[Cronet] Native CronetEngine API should allow setting of network thread priority.

Project Member Reported by mef@chromium.org, Jul 31

Issue description

The Network Thread priority API is available on both Android and iOS and provides improved performance in some applications. 

Native CronetEngine API should provide same functionality.



 
Blocking: 786559
Hmm there appear to be various thread priority APIs that take different values:
iOS takes a double value from 0.0 to 1.0.
Android takes an int from -20 to 19.
base/threading/thread.h takes one of four values.
I generally think the Cronet API should be as powerful as possible; and users wanting a simpler API should use a wrapper.  So I'm tempted to say we should accept a double whose value means:
1. On Android it should be an integer between -20 and 19
2. On iOS it should be a double between 0.0 and 1.0
3. On other OS's it should be one of the four values in platform_thread.h

Presently the thread priority is set by the embedder through the Android-specific and iOS-specific APIs and set in the non-shared Cronet impl code.  Perhaps we should transfer to a mode where the thread priority is passed through the cronet::URLRequestContextConfig API and set accordingly by shared Cronet code (though using OS-specific impls).
I agree with adding a double network_thread_priority value to Cronet EngineParams and specifying that valid range is platform-specific.

We can define the range on Android and iOS and ignore on other platforms.

We should also define some out-of-range value that means 'use default'.
Owner: pauljensen@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd

commit 6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd
Author: Paul Jensen <pauljensen@chromium.org>
Date: Fri Aug 24 14:46:41 2018

[Cronet] Add ability to set network thread priority to native API

Do so by sharing logic from Android and iOS.

Bug:  869406 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: If51d4acb13103818171bbd9e3f6ddc0e7bb9e294
Reviewed-on: https://chromium-review.googlesource.com/1185210
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585836}
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/android/cronet_library_loader.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/android/java/src/org/chromium/net/impl/CronetLibraryLoader.java
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/cronet_global_state.h
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/cronet_global_state_stubs.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/cronet_url_request_context.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/ios/cronet_environment.mm
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/ios/cronet_global_state_ios.mm
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/native/cronet.idl
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/native/engine.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/native/generated/cronet.idl_c.h
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/native/generated/cronet.idl_impl_struct.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/native/generated/cronet.idl_impl_struct.h
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/native/generated/cronet.idl_impl_struct_unittest.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/stale_host_resolver_unittest.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/tools/generators/c_templates/module_impl_struct_unittest.cc.tmpl
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/url_request_context_config.cc
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/url_request_context_config.h
[modify] https://crrev.com/6a1ea3ac8e0d2a4619207345ea8aba022a5b0ffd/components/cronet/url_request_context_config_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment