New issue
Advanced search Search tips

Issue 785460 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

base::ios::ScopedCriticalAction poor performance

Reported by kuznetso...@gmail.com, Nov 15 2017

Issue description

Steps to reproduce the problem:
This code snippet generates multiple critical actions.

// Copyright (c) 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/ios/scoped_critical_action.h"

#include "base/bind.h"
#include "base/critical_closure.h"
#include "base/synchronization/waitable_event.h"
#include "base/threading/thread.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace base {
namespace ios {

TEST(ScopedCriticalAction, PerformanceProfile) {
  Thread thread("BackgroundThread");
  thread.Start();

  const int kIterations = 10000;
  int counter = 0;

  for (int i = 0; i < kIterations; ++i) {
    thread.task_runner()->PostTask(
        FROM_HERE,
        MakeCriticalClosure(BindOnce([](int* counter) { ++(*counter); },
                                     Unretained(&counter))));
  }

  WaitableEvent event(WaitableEvent::ResetPolicy::MANUAL,
                      WaitableEvent::InitialState::NOT_SIGNALED);
  thread.task_runner()->PostTask(
      FROM_HERE, BindOnce([](WaitableEvent* event) { event->Signal(); },
                          Unretained(&event)));

  event.Wait();

  EXPECT_EQ(kIterations, counter);
}

}  // namespace ios
}  // namespace base

Launch test on iOS 11 simulator:
out/Profile-iphonesimulator/iossim -s 11.1 -d 'iPhone 6' -c '--gtest_filter=ScopedCriticalAction.PerformanceProfile' out/Profile-iphonesimulator/base_unittests.app

Running time:
[----------] Global test environment set-up.
[----------] 1 test from ScopedCriticalAction
[ RUN      ] ScopedCriticalAction.PerformanceProfile
[       OK ] ScopedCriticalAction.PerformanceProfile (36101 ms)
[----------] 1 test from ScopedCriticalAction (36102 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (36103 ms total)
[  PASSED  ] 1 test.

Launch the same code without MakeCriticalClosure:
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ScopedCriticalAction
[ RUN      ] ScopedCriticalAction.PerformanceProfile
[       OK ] ScopedCriticalAction.PerformanceProfile (44 ms)
[----------] 1 test from ScopedCriticalAction (46 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (46 ms total)
[  PASSED  ] 1 test.

What is the expected behavior?

What went wrong?
base::ios::ScopedCriticalAction performance is poor and should be improved.

Did this work before? N/A 

Chrome version: 64  Channel: n/a
OS Version: iOS 11
Flash Version: Shockwave Flash 27.0 r0
 
Profiler screenshots.
begin_background_task.png
185 KB View Download
end_background.png
111 KB View Download
How does this compare with the changes proposed in https://chromium-review.googlesource.com/c/chromium/src/+/771250?
Components: Internals
Owner: sdefresne@chromium.org
Status: Assigned (was: Unconfirmed)
Sorry, I've used wrong account for issue reporting.

After https://chromium-review.googlesource.com/c/chromium/src/+/771250 code snippet performs just like code without MakeCriticalClosure

[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ScopedCriticalAction
[ RUN      ] ScopedCriticalAction.PerformanceProfile
[       OK ] ScopedCriticalAction.PerformanceProfile (44 ms)
[----------] 1 test from ScopedCriticalAction (46 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (46 ms total)
[  PASSED  ] 1 test.

Proposed CL improves base::ios::ScopedCriticalAction performance when multiple "critial actions" exists at the same time.

Sign in to add a comment