Skip to content

Commit

Permalink
Move OneWriterSeqLock and SharedMemorySeqLockBuffer from content/ to …
Browse files Browse the repository at this point in the history
…device/base/synchronization

The reason to move it to device/base/synchronization is that it is going to be used both for content/ Device Orientation and Gamepad and for device/ Generic Sensors.

Review-Url: https://codereview.chromium.org/2358123005
Cr-Commit-Position: refs/heads/master@{#422115}
  • Loading branch information
darktears authored and Commit bot committed Sep 30, 2016
1 parent 10c6fdc commit b39a308
Show file tree
Hide file tree
Showing 17 changed files with 83 additions and 57 deletions.
3 changes: 1 addition & 2 deletions content/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ source_set("common") {
"net/url_request_service_worker_data.h",
"net/url_request_user_data.cc",
"net/url_request_user_data.h",
"one_writer_seqlock.cc",
"one_writer_seqlock.h",
"origin_trials/trial_token.cc",
"origin_trials/trial_token.h",
"origin_trials/trial_token_validator.cc",
Expand Down Expand Up @@ -368,6 +366,7 @@ source_set("common") {
"//components/tracing:startup_tracing",
"//content:resources",
"//content/app/resources",
"//device/base/synchronization",
"//device/bluetooth",
"//gpu",
"//gpu/command_buffer/client:gles2_c_lib",
Expand Down
1 change: 1 addition & 0 deletions content/common/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include_rules = [
"-storage/browser",

"+device/base/synchronization",
"+services/shell/public/cpp",

# No inclusion of WebKit from the browser, other than strictly enum/POD,
Expand Down
5 changes: 3 additions & 2 deletions content/common/device_sensors/device_light_hardware_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
#define CONTENT_COMMON_DEVICE_SENSORS_DEVICE_LIGHT_HARDWARE_BUFFER_H_

#include "content/common/device_sensors/device_light_data.h"
#include "content/common/shared_memory_seqlock_buffer.h"
#include "device/base/synchronization/shared_memory_seqlock_buffer.h"

namespace content {

typedef SharedMemorySeqLockBuffer<DeviceLightData> DeviceLightHardwareBuffer;
typedef device::SharedMemorySeqLockBuffer<DeviceLightData>
DeviceLightHardwareBuffer;

} // namespace content

Expand Down
4 changes: 2 additions & 2 deletions content/common/device_sensors/device_motion_hardware_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
#ifndef CONTENT_COMMON_DEVICE_SENSORS_DEVICE_MOTION_HARDWARE_BUFFER_H_
#define CONTENT_COMMON_DEVICE_SENSORS_DEVICE_MOTION_HARDWARE_BUFFER_H_

#include "content/common/shared_memory_seqlock_buffer.h"
#include "device/base/synchronization/shared_memory_seqlock_buffer.h"
#include "third_party/WebKit/public/platform/modules/device_orientation/WebDeviceMotionData.h"

namespace content {

typedef SharedMemorySeqLockBuffer<blink::WebDeviceMotionData>
typedef device::SharedMemorySeqLockBuffer<blink::WebDeviceMotionData>
DeviceMotionHardwareBuffer;

} // namespace content
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
#ifndef CONTENT_COMMON_DEVICE_SENSORS_DEVICE_ORIENTATION_HARDWARE_BUFFER_H_
#define CONTENT_COMMON_DEVICE_SENSORS_DEVICE_ORIENTATION_HARDWARE_BUFFER_H_

#include "content/common/shared_memory_seqlock_buffer.h"
#include "device/base/synchronization/shared_memory_seqlock_buffer.h"
#include "third_party/WebKit/public/platform/modules/device_orientation/WebDeviceOrientationData.h"

namespace content {

typedef SharedMemorySeqLockBuffer<blink::WebDeviceOrientationData>
typedef device::SharedMemorySeqLockBuffer<blink::WebDeviceOrientationData>
DeviceOrientationHardwareBuffer;

} // namespace content
Expand Down
4 changes: 2 additions & 2 deletions content/common/gamepad_hardware_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#ifndef CONTENT_COMMON_GAMEPAD_HARDWARE_BUFFER_H_
#define CONTENT_COMMON_GAMEPAD_HARDWARE_BUFFER_H_

#include "content/common/one_writer_seqlock.h"
#include "device/base/synchronization/one_writer_seqlock.h"
#include "third_party/WebKit/public/platform/WebGamepads.h"

namespace content {
Expand All @@ -25,7 +25,7 @@ contention is detected by using the associated SeqLock.

struct GamepadHardwareBuffer {
// FIXME: Use the generic SharedMemorySeqLockBuffer<blink::WebGamepads>.
OneWriterSeqLock sequence;
device::OneWriterSeqLock sequence;
blink::WebGamepads buffer;
};

Expand Down
1 change: 1 addition & 0 deletions content/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ target(link_target_type, "renderer") {
"//content/public/common:feature_h264_with_openh264_ffmpeg",
"//content/public/common:features",
"//crypto:platform",
"//device/base/synchronization",
"//device/battery:mojo_bindings",
"//device/bluetooth",
"//device/time_zone_monitor/public/interfaces",
Expand Down
1 change: 1 addition & 0 deletions content/renderer/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ include_rules = [
"+content/child",
"+device/battery", # For battery status service.
"+device/sensors/public",
"+device/base/synchronization",
"+device/time_zone_monitor/public",
"+device/usb/public",
"+device/vibration",
Expand Down
6 changes: 4 additions & 2 deletions content/renderer/shared_memory_seqlock_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ SharedMemorySeqLockReaderBase::InitializeSharedMemory(
}

bool SharedMemorySeqLockReaderBase::FetchFromBuffer(
content::OneWriterSeqLock* seqlock, void* final, void* temp, void* from,
device::OneWriterSeqLock* seqlock,
void* final,
void* temp,
void* from,
size_t size) {

if (!base::SharedMemory::IsHandleValid(renderer_shared_memory_handle_))
return false;

Expand Down
16 changes: 10 additions & 6 deletions content/renderer/shared_memory_seqlock_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/shared_memory.h"
#include "content/common/shared_memory_seqlock_buffer.h"
#include "device/base/synchronization/shared_memory_seqlock_buffer.h"

namespace content {
namespace internal {
Expand All @@ -26,8 +26,11 @@ class SharedMemorySeqLockReaderBase {
base::SharedMemoryHandle shared_memory_handle,
size_t buffer_size);

bool FetchFromBuffer(content::OneWriterSeqLock* seqlock, void* final,
void* temp, void* from, size_t size);
bool FetchFromBuffer(device::OneWriterSeqLock* seqlock,
void* final,
void* temp,
void* from,
size_t size);

static const int kMaximumContentionCount = 10;
base::SharedMemoryHandle renderer_shared_memory_handle_;
Expand All @@ -54,16 +57,17 @@ class SharedMemorySeqLockReader

bool Initialize(base::SharedMemoryHandle shared_memory_handle) {
if (void* memory = InitializeSharedMemory(
shared_memory_handle, sizeof(SharedMemorySeqLockBuffer<Data>))) {
buffer_ = static_cast<SharedMemorySeqLockBuffer<Data>*>(memory);
shared_memory_handle,
sizeof(device::SharedMemorySeqLockBuffer<Data>))) {
buffer_ = static_cast<device::SharedMemorySeqLockBuffer<Data>*>(memory);
temp_buffer_.reset(new Data);
return true;
}
return false;
}

private:
SharedMemorySeqLockBuffer<Data>* buffer_;
device::SharedMemorySeqLockBuffer<Data>* buffer_;
std::unique_ptr<Data> temp_buffer_;

DISALLOW_COPY_AND_ASSIGN(SharedMemorySeqLockReader);
Expand Down
3 changes: 2 additions & 1 deletion content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ static_library("test_support") {
"//content/renderer:for_content_tests",
"//content/shell:pak",
"//content/utility:for_content_tests",
"//device/base/synchronization",
"//device/geolocation",
"//device/nfc:mojo_bindings",
"//ipc:test_support",
Expand Down Expand Up @@ -721,6 +722,7 @@ test("content_browsertests") {
"//content/shell:content_shell_lib",
"//content/shell:pak",
"//content/test:test_support",
"//device/base/synchronization",
"//device/battery",
"//device/battery:mojo_bindings",
"//device/power_save_blocker",
Expand Down Expand Up @@ -1281,7 +1283,6 @@ test("content_unittests") {
"../common/mac/font_descriptor_unittest.mm",
"../common/manifest_util_unittest.cc",
"../common/navigation_params_unittest.cc",
"../common/one_writer_seqlock_unittest.cc",
"../common/origin_trials/trial_token_unittest.cc",
"../common/origin_trials/trial_token_validator_unittest.cc",
"../common/origin_util_unittest.cc",
Expand Down
3 changes: 3 additions & 0 deletions device/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ is_linux_without_udev = is_linux && !use_udev

test("device_unittests") {
sources = [
"base/synchronization/one_writer_seqlock_unittest.cc",
"battery/battery_status_manager_win_unittest.cc",
"battery/battery_status_service_unittest.cc",
"bluetooth/bluetooth_adapter_mac_unittest.mm",
Expand Down Expand Up @@ -66,6 +67,8 @@ test("device_unittests") {

deps = [
"//base/test:test_support",
"//base/third_party/dynamic_annotations:dynamic_annotations",
"//device/base/synchronization",
"//device/battery",
"//device/battery:mojo_bindings",
"//device/bluetooth",
Expand Down
17 changes: 17 additions & 0 deletions device/base/synchronization/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2014 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.

import("//build/config/features.gni")

source_set("synchronization") {
sources = [
"one_writer_seqlock.cc",
"one_writer_seqlock.h",
"shared_memory_seqlock_buffer.h",
]

deps = [
"//base",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/common/one_writer_seqlock.h"
#include "device/base/synchronization/one_writer_seqlock.h"

namespace content {
namespace device {

OneWriterSeqLock::OneWriterSeqLock()
: sequence_(0) {
}
OneWriterSeqLock::OneWriterSeqLock() : sequence_(0) {}

base::subtle::Atomic32 OneWriterSeqLock::ReadBegin() {
base::subtle::Atomic32 version;
Expand Down Expand Up @@ -46,4 +44,4 @@ void OneWriterSeqLock::WriteEnd() {
base::subtle::Barrier_AtomicIncrement(&sequence_, 1);
}

} // namespace content
} // namespace device
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_COMMON_ONE_WRITER_SEQLOCK_H_
#define CONTENT_COMMON_ONE_WRITER_SEQLOCK_H_
#ifndef DEVICE_BASE_SYNCHRONIZATION_ONE_WRITER_SEQLOCK_H_
#define DEVICE_BASE_SYNCHRONIZATION_ONE_WRITER_SEQLOCK_H_

#include "base/atomicops.h"
#include "base/macros.h"
#include "base/threading/platform_thread.h"
#include "content/common/content_export.h"

namespace content {
namespace device {

// This SeqLock handles only *one* writer and multiple readers. It may be
// suitable for low-contention with relatively infrequent writes, and many
Expand All @@ -29,7 +28,7 @@ namespace content {
// garbage, or indices could be out of range. Probably the only suitable thing
// to do during the read loop is to make a copy of the data, and operate on it
// only after the read was found to be consistent.
class CONTENT_EXPORT OneWriterSeqLock {
class OneWriterSeqLock {
public:
OneWriterSeqLock();
base::subtle::Atomic32 ReadBegin();
Expand All @@ -42,6 +41,6 @@ class CONTENT_EXPORT OneWriterSeqLock {
DISALLOW_COPY_AND_ASSIGN(OneWriterSeqLock);
};

} // namespace content
} // namespace device

#endif // CONTENT_COMMON_ONE_WRITER_SEQLOCK_H_
#endif // DEVICE_BASE_SYNCHRONIZATION_ONE_WRITER_SEQLOCK_H_
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/common/one_writer_seqlock.h"
#include "device/base/synchronization/one_writer_seqlock.h"

#include <stdlib.h>

Expand All @@ -13,29 +13,28 @@
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace base {
namespace device {

// Basic test to make sure that basic operation works correctly.

struct TestData {
unsigned a, b, c;
};

class BasicSeqLockTestThread : public PlatformThread::Delegate {
class BasicSeqLockTestThread : public base::PlatformThread::Delegate {
public:
BasicSeqLockTestThread() {}

void Init(
content::OneWriterSeqLock* seqlock,
TestData* data,
base::AtomicRefCount* ready) {
void Init(OneWriterSeqLock* seqlock,
TestData* data,
base::AtomicRefCount* ready) {
seqlock_ = seqlock;
data_ = data;
ready_ = ready;
}
void ThreadMain() override {
while (AtomicRefCountIsZero(ready_)) {
PlatformThread::YieldCurrentThread();
while (base::AtomicRefCountIsZero(ready_)) {
base::PlatformThread::YieldCurrentThread();
}

for (unsigned i = 0; i < 1000; ++i) {
Expand All @@ -50,11 +49,11 @@ class BasicSeqLockTestThread : public PlatformThread::Delegate {
EXPECT_EQ(copy.c, copy.b + copy.a);
}

AtomicRefCountDec(ready_);
base::AtomicRefCountDec(ready_);
}

private:
content::OneWriterSeqLock* seqlock_;
OneWriterSeqLock* seqlock_;
TestData* data_;
base::AtomicRefCount* ready_;

Expand All @@ -67,20 +66,20 @@ class BasicSeqLockTestThread : public PlatformThread::Delegate {
#define MAYBE_ManyThreads ManyThreads
#endif
TEST(OneWriterSeqLockTest, MAYBE_ManyThreads) {
content::OneWriterSeqLock seqlock;
TestData data = { 0, 0, 0 };
OneWriterSeqLock seqlock;
TestData data = {0, 0, 0};
base::AtomicRefCount ready = 0;

ANNOTATE_BENIGN_RACE_SIZED(&data, sizeof(data), "Racey reads are discarded");

static const unsigned kNumReaderThreads = 10;
BasicSeqLockTestThread threads[kNumReaderThreads];
PlatformThreadHandle handles[kNumReaderThreads];
base::PlatformThreadHandle handles[kNumReaderThreads];

for (unsigned i = 0; i < kNumReaderThreads; ++i)
threads[i].Init(&seqlock, &data, &ready);
for (unsigned i = 0; i < kNumReaderThreads; ++i)
ASSERT_TRUE(PlatformThread::Create(0, &threads[i], &handles[i]));
ASSERT_TRUE(base::PlatformThread::Create(0, &threads[i], &handles[i]));

// The main thread is the writer, and the spawned are readers.
unsigned counter = 0;
Expand All @@ -94,12 +93,12 @@ TEST(OneWriterSeqLockTest, MAYBE_ManyThreads) {
if (counter == 1)
base::AtomicRefCountIncN(&ready, kNumReaderThreads);

if (AtomicRefCountIsZero(&ready))
if (base::AtomicRefCountIsZero(&ready))
break;
}

for (unsigned i = 0; i < kNumReaderThreads; ++i)
PlatformThread::Join(handles[i]);
base::PlatformThread::Join(handles[i]);
}

} // namespace base
} // namespace device
Loading

0 comments on commit b39a308

Please sign in to comment.