From 8797516daa4b465022ee42ce57a8bb40ed53316d Mon Sep 17 00:00:00 2001 From: erikchen Date: Wed, 28 Oct 2015 12:47:29 -0700 Subject: reland 1: "mac: Run A/B experiment on SharedMemory mechanism for resource buffers." The first CL used a new UMA feature FeatureList. FeatureList is not yet ready for production use, so I've switched back to FieldTrial. > The CL changes the default behavior of resource buffer to use a Mach-backed > SharedMemory region. This CL adds a field trial to measure the effect of Mach vs > POSIX backed SharedMemory regions. > > BUG=547261, 466437 > Committed: https://crrev.com/e0e2fd398f3d07c8eebbe662d71a0f9286862476 > Cr-Commit-Position: refs/heads/master@{#356513} BUG=547261, 466437 TBR=mark@chromium.org, isherman@chromium.org, asvitkine@chromium.org, avi@chromium.org Review URL: https://codereview.chromium.org/1412923003 Cr-Commit-Position: refs/heads/master@{#356620} --- base/memory/shared_memory.h | 4 +++ base/memory/shared_memory_handle.h | 3 ++ base/memory/shared_memory_mac.cc | 47 +++++++++++++++++++++++++++++++ content/browser/loader/resource_buffer.cc | 4 +++ tools/metrics/histograms/histograms.xml | 18 ++++++++++++ 5 files changed, 76 insertions(+) diff --git a/base/memory/shared_memory.h b/base/memory/shared_memory.h index 18416c0..5cdcb0d 100644 --- a/base/memory/shared_memory.h +++ b/base/memory/shared_memory.h @@ -140,6 +140,10 @@ class BASE_EXPORT SharedMemory { // http://crbug.com/466437. bool CreateAndMapAnonymousPosix(size_t size); bool CreateAnonymousPosix(size_t size); + + // This method is an analog of CreateAndMapAnonymous that forces the + // underlying OS primitive to be a Mach memory object. + bool CreateAndMapAnonymousMach(size_t size); #endif // defined(OS_MACOSX) && !defined(OS_IOS) // Creates an anonymous shared memory segment of size size. diff --git a/base/memory/shared_memory_handle.h b/base/memory/shared_memory_handle.h index b3dfc8f..49398a4 100644 --- a/base/memory/shared_memory_handle.h +++ b/base/memory/shared_memory_handle.h @@ -75,12 +75,15 @@ class BASE_EXPORT SharedMemoryHandle { #else class BASE_EXPORT SharedMemoryHandle { public: + // The values of these enums must not change, as they are used by the + // histogram OSX.SharedMemory.Mechanism. enum Type { // The SharedMemoryHandle is backed by a POSIX fd. POSIX, // The SharedMemoryHandle is backed by the Mach primitive "memory object". MACH, }; + static const int TypeMax = 2; // The format that should be used to transmit |Type| over the wire. typedef int TypeWireFormat; diff --git a/base/memory/shared_memory_mac.cc b/base/memory/shared_memory_mac.cc index 799b8e3..47b8a89 100644 --- a/base/memory/shared_memory_mac.cc +++ b/base/memory/shared_memory_mac.cc @@ -14,6 +14,8 @@ #include "base/files/scoped_file.h" #include "base/logging.h" #include "base/mac/scoped_mach_vm.h" +#include "base/metrics/field_trial.h" +#include "base/metrics/histogram_macros.h" #include "base/posix/eintr_wrapper.h" #include "base/posix/safe_strerror.h" #include "base/process/process_metrics.h" @@ -29,6 +31,38 @@ namespace base { namespace { +const char kTrialName[] = "MacMemoryMechanism"; +const char kTrialMach[] = "Mach"; +const char kTrialPosix[] = "Posix"; + +SharedMemoryHandle::Type GetABTestMechanism() { + static bool found_group = false; + static SharedMemoryHandle::Type group = SharedMemoryHandle::MACH; + + if (found_group) + return group; + + const std::string group_name = + base::FieldTrialList::FindFullName(kTrialName); + if (group_name == kTrialMach) { + group = SharedMemoryHandle::MACH; + found_group = true; + } else if (group_name == kTrialPosix) { + group = SharedMemoryHandle::POSIX; + found_group = true; + } else { + group = SharedMemoryHandle::MACH; + } + + return group; +} + +// Emits a histogram entry indicating which type of SharedMemory was created. +void EmitMechanism(SharedMemoryHandle::Type type) { + UMA_HISTOGRAM_ENUMERATION("OSX.SharedMemory.Mechanism", type, + SharedMemoryHandle::TypeMax); +} + // Returns whether the operation succeeded. // |new_handle| is an output variable, populated on success. The caller takes // ownership of the underlying memory object. @@ -227,6 +261,17 @@ bool SharedMemory::CreateAnonymousPosix(size_t size) { return Create(options); } +bool SharedMemory::CreateAndMapAnonymousMach(size_t size) { + SharedMemoryCreateOptions options; + + // A/B test the mechanism. Once the experiment is over, this will always be + // set to SharedMemoryHandle::MACH. + // http://crbug.com/547261 + options.type = GetABTestMechanism(); + options.size = size; + return Create(options) && Map(size); +} + // static bool SharedMemory::GetSizeFromSharedMemoryHandle( const SharedMemoryHandle& handle, @@ -248,6 +293,8 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) { if (options.size > static_cast(std::numeric_limits::max())) return false; + EmitMechanism(options.type); + if (options.type == SharedMemoryHandle::MACH) { shm_ = SharedMemoryHandle(options.size); requested_size_ = options.size; diff --git a/content/browser/loader/resource_buffer.cc b/content/browser/loader/resource_buffer.cc index 9dc63d6..58839db 100644 --- a/content/browser/loader/resource_buffer.cc +++ b/content/browser/loader/resource_buffer.cc @@ -54,7 +54,11 @@ bool ResourceBuffer::Initialize(int buffer_size, min_alloc_size_ = min_allocation_size; max_alloc_size_ = max_allocation_size; +#if defined(OS_MACOSX) && !defined(OS_IOS) + return shared_mem_.CreateAndMapAnonymousMach(buf_size_); +#else return shared_mem_.CreateAndMapAnonymous(buf_size_); +#endif // defined(OS_MACOSX) && !defined(OS_IOS) } bool ResourceBuffer::IsInitialized() const { diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 2a2add0..dd54238 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -30036,6 +30036,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + erikchen@chromium.org + + A histogram entry is emitted each time a base::SharedMemory object is + constructed. The value of the entry indicates the mechanism used to back the + shared memory region. + + + erikchen@chromium.org @@ -67977,6 +67986,15 @@ To add a new entry, add it with any value and run test to compute valid value. + + + The shared memory region is backed by a POSIX fd. + + + The shared memory region is backed by a Mach memory object. + + + -- cgit v1.1