Skip to content

Commit

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

Reason for revert: The added unit tests fail on the Linux CFI bot 
Failing tests: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20CFI/25348/overview

Original change's description:
> Reland "pthreadpool: Integrate with Jobs API"
>
> This is a reland of commit e294652
>
> This reland fixes the build issue on win-arm64 bots, such as [1], by
> declaring the missing arm64 intrinsics if the arm64intr.h is not
> included correctly due to LLVM issue [2]. Once the LLVM issue is
> resolved, this work-around should be removed.
>
> [1]: https://ci.chromium.org/b/8779781271284246545
> [2]: llvm/llvm-project#62942
>
> 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: Icab18064790a70164e47b52939add5774bf49ec7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4571505
> Commit-Queue: ningxin hu <ningxin.hu@intel.com>
> Reviewed-by: Robert Ogden <robertogden@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Austin Sullivan <asully@chromium.org>
> Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1153620}

Bug: 1228275,1273291
Change-Id: I07ffdcb24333cc7cdb3c4ad78eca50d66d4690c1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4590575
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Fabian Sommer <fabiansommer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1153770}
  • Loading branch information
@Fabian-Sommer
Fabian-Sommer authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent 2ad8d72 commit 80fb8ec
Show file tree
Hide file tree
Showing 20 changed files with 8 additions and 1,470 deletions.
4 changes: 0 additions & 4 deletions infra/config/generated/testing/gn_isolate_map.pyl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1433,10 +1433,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1755,11 +1755,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
View file
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
View file
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 80fb8ec

Please sign in to comment.