Skip to content

Commit

Permalink
Revert "pthreadpool: Integrate with Jobs API"
Browse files Browse the repository at this point in the history
This reverts commit e294652.

Reason for revert:
LUCI Bisection identified this CL as the culprit of a build failure. See the analysis: https://luci-bisection.appspot.com/analysis/b/8779781271284246545

Sample failed build: https://ci.chromium.org/b/8779781271284246545

If this is a false positive, please report it at https://bugs.chromium.org/p/chromium/issues/entry?comment=Analysis%3A+https%3A%2F%2Fluci-bisection.appspot.com%2Fanalysis%2Fb%2F8779781271284246545&components=Tools%3ETest%3EFindit&labels=LUCI-Bisection-Wrong%2CPri-3%2CType-Bug&status=Available&summary=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F4467727

Original change's description:
> pthreadpool: Integrate with Jobs API
>
> pthreadpool is used by XNNPACK which in turns supports TFLite and WebNN
> to execute each neural network operator of a model by multiple threads
> in parallel.
>
> To avoid creating an unmanaged thread pool owned by pthreadpool
> implementation, this CL implements a shim of `pthreadpool_parallelize()`
> that maps the pthreadpool thread tasks to work items and uses Jobs API
> (`base::PostJob`) to schedule these work items with Chromium's
> `base::ThreadPool` workers.
>
> According to the test result on a multi-core device, when scheduling 4
> work items with `base::ThreadPool`, XNNPACK model inference performance
> could get ~1.9X speedup for MobileNet V2 and ~2.8X speedup for ResNet50
> V2 compared to single thread execution. And the test result also shows
> using Jobs API shim could achieve 85% and 97% performance of using
> pthreadpool own thread pool implementation for MobileNet V2 and ResNet50
> V2 respectively.
>
> This CL also enables pthreadpool_unittests with Jobs API integration on
> Windows and Linux bots that aligns with the `build_tflite_with_xnnpack`
> build flag.
>
>
> Bug: 1228275,1273291
> Change-Id: I1152d1e93885399b453c87be18a432e6c118054e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4467727
> Reviewed-by: danakj <danakj@chromium.org>
> Commit-Queue: ningxin hu <ningxin.hu@intel.com>
> Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> Reviewed-by: Robert Ogden <robertogden@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Reviewed-by: Austin Sullivan <asully@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1150206}
>

Bug: 1228275,1273291
Change-Id: I60ce5354a3602eeaae27557d17d91017b940b137
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4567437
Commit-Queue: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Owners-Override: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Bot-Commit: luci-bisection@appspot.gserviceaccount.com <luci-bisection@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1150225}
  • Loading branch information
luci-bisection@appspot.gserviceaccount.com authored and Chromium LUCI CQ committed May 29, 2023
1 parent 03c5556 commit 06d5c4b
Show file tree
Hide file tree
Showing 20 changed files with 8 additions and 1,422 deletions.
4 changes: 0 additions & 4 deletions infra/config/generated/testing/gn_isolate_map.pyl
Original file line number Diff line number Diff line change
Expand Up @@ -1440,10 +1440,6 @@
"label": "//chrome/browser/metrics/perf:profile_provider_unittest",
"type": "console_test_launcher",
},
"pthreadpool_unittests": {
"label": "//third_party/pthreadpool:pthreadpool_unittests",
"type": "console_test_launcher",
},
"push_apps_to_background_apk": {
"label": "//tools/android/push_apps_to_background:push_apps_to_background_apk",
"type": "additional_compile_target",
Expand Down
5 changes: 0 additions & 5 deletions infra/config/targets/targets.star
Original file line number Diff line number Diff line change
Expand Up @@ -1763,11 +1763,6 @@ targets.console_test_launcher(
label = "//chrome/browser/metrics/perf:profile_provider_unittest",
)

targets.console_test_launcher(
name = "pthreadpool_unittests",
label = "//third_party/pthreadpool:pthreadpool_unittests",
)

targets.compile_target(
name = "push_apps_to_background_apk",
label = "//tools/android/push_apps_to_background:push_apps_to_background_apk",
Expand Down
19 changes: 0 additions & 19 deletions testing/buildbot/chrome.json
Original file line number Diff line number Diff line change
Expand Up @@ -3388,25 +3388,6 @@
"test": "printing_unittests",
"test_id_prefix": "ninja://printing:printing_unittests/"
},
{
"ci_only": true,
"merge": {
"script": "//testing/merge_scripts/standard_gtest_merge.py"
},
"swarming": {
"can_use_on_swarming_builders": true,
"dimension_sets": [
{
"os": "Ubuntu-22.04",
"pool": "chrome.tests",
"ssd": "0"
}
],
"service_account": "chrome-tester@chops-service-accounts.iam.gserviceaccount.com"
},
"test": "pthreadpool_unittests",
"test_id_prefix": "ninja://third_party/pthreadpool:pthreadpool_unittests/"
},
{
"merge": {
"script": "//testing/merge_scripts/standard_gtest_merge.py"
Expand Down
56 changes: 0 additions & 56 deletions testing/buildbot/chromium.cft.json
Original file line number Diff line number Diff line change
Expand Up @@ -1186,25 +1186,6 @@
"test": "printing_unittests",
"test_id_prefix": "ninja://printing:printing_unittests/"
},
{
"ci_only": true,
"isolate_profile_data": true,
"merge": {
"script": "//testing/merge_scripts/standard_gtest_merge.py"
},
"swarming": {
"can_use_on_swarming_builders": true,
"dimension_sets": [
{
"cpu": "arm64",
"os": "Ubuntu-18.04"
}
],
"service_account": "chromium-tester@chops-service-accounts.iam.gserviceaccount.com"
},
"test": "pthreadpool_unittests",
"test_id_prefix": "ninja://third_party/pthreadpool:pthreadpool_unittests/"
},
{
"isolate_profile_data": true,
"merge": {
Expand Down Expand Up @@ -3387,24 +3368,6 @@
"test": "printing_unittests",
"test_id_prefix": "ninja://printing:printing_unittests/"
},
{
"ci_only": true,
"isolate_profile_data": true,
"merge": {
"script": "//testing/merge_scripts/standard_gtest_merge.py"
},
"swarming": {
"can_use_on_swarming_builders": true,
"dimension_sets": [
{
"os": "Ubuntu-18.04"
}
],
"service_account": "chromium-tester@chops-service-accounts.iam.gserviceaccount.com"
},
"test": "pthreadpool_unittests",
"test_id_prefix": "ninja://third_party/pthreadpool:pthreadpool_unittests/"
},
{
"isolate_profile_data": true,
"merge": {
Expand Down Expand Up @@ -7493,25 +7456,6 @@
"test": "printing_unittests",
"test_id_prefix": "ninja://printing:printing_unittests/"
},
{
"ci_only": true,
"isolate_profile_data": true,
"merge": {
"script": "//testing/merge_scripts/standard_gtest_merge.py"
},
"swarming": {
"can_use_on_swarming_builders": true,
"dimension_sets": [
{
"cpu": "x86-64",
"os": "Windows-10-19045"
}
],
"service_account": "chromium-tester@chops-service-accounts.iam.gserviceaccount.com"
},
"test": "pthreadpool_unittests",
"test_id_prefix": "ninja://third_party/pthreadpool:pthreadpool_unittests/"
},
{
"isolate_profile_data": true,
"merge": {
Expand Down

0 comments on commit 06d5c4b

Please sign in to comment.