base: Discardable memory types.

base/memory/discardable_memory.cc

Index: base/memory/discardable_memory.cc
diff --git a/base/memory/discardable_memory.cc b/base/memory/discardable_memory.cc
new file mode 100644
index 0000000000000000000000000000000000000000..dcecc0f665ebbd256f49fb64d1412ea756dde01f
--- /dev/null
+++ b/base/memory/discardable_memory.cc
@@ -0,0 +1,66 @@
+// Copyright 2013 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/memory/discardable_memory.h"
+
+#include "base/lazy_instance.h"
+
+namespace base {
+namespace {
+
+const struct {
+  const char* name;
+  DiscardableMemoryType type;
+} kTypeNamePairs[] = {
+  { "android", DISCARDABLE_MEMORY_TYPE_ANDROID },
+  { "mac", DISCARDABLE_MEMORY_TYPE_MAC },
+  { "emulated", DISCARDABLE_MEMORY_TYPE_EMULATED }
+};
+
+struct TypeInitializedWithPreferredValue {
+  TypeInitializedWithPreferredValue() : value(DISCARDABLE_MEMORY_TYPE_NONE) {
+    std::vector<DiscardableMemoryType> supported_types;
+    DiscardableMemory::GetSupportedTypes(&supported_types);
+    DCHECK(!supported_types.empty());
+    value = supported_types[0];
+  }
+  DiscardableMemoryType value;
+};
+LazyInstance<TypeInitializedWithPreferredValue>::Leaky
+    g_discardable_memory_type = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+// static
+DiscardableMemoryType DiscardableMemory::GetNamedType(
+    const std::string& name) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTypeNamePairs); ++i) {

[willchan no longer on Chromium, 2013/12/24 01:21:06]

I don't think you need to use the unsafe version of arraysize here.

[reveman, 2013/12/26 11:46:08]

Done. After after giving the array type a name.

+    if (name == kTypeNamePairs[i].name)
+      return kTypeNamePairs[i].type;
+  }
+
+  return DISCARDABLE_MEMORY_TYPE_NONE;
+}
+
+// static
+const char* DiscardableMemory::GetTypeName(DiscardableMemoryType type) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTypeNamePairs); ++i) {

[willchan no longer on Chromium, 2013/12/24 01:21:06]

ditto...i think you can switch to arraysize()

[reveman, 2013/12/26 11:46:08]

Done.

+    if (type == kTypeNamePairs[i].type)
+      return kTypeNamePairs[i].name;
+  }
+
+  return "unknown";
+}
+
+// static
+void DiscardableMemory::SetType(DiscardableMemoryType type) {
+  g_discardable_memory_type.Get().value = type;

[willchan no longer on Chromium, 2013/12/24 01:21:06]

This looks racy to me. If this is only allowed to be set once on startup, you
should change it so that value is only allowed to be set before the first
GetType() (and any attempt afterward is a bug and should fire a DCHECK()).
Otherwise, if it's allowed to be set at runtime, use a Lock. Or if you really
think performance is important (yada yada about premature optimization), use an
atomic. FWIW, I think we should only let this get set on startup.

Not sure if you've seen it before, but let me speculatively save a codereview
roundtrip by saying that there's no such thing as a benign data race:
https://www.usenix.org/legacy/event/hotpar11/tech/final_files/Boehm.pdf.

[reveman, 2013/12/26 11:46:08]

Unit tests depend on being able to change this at runtime. Setting it once at
startup is fine elsewhere though.

I was planning to leave the thread safety responsibility up to the client so the
unit test code can do what it currently does. Latest patch does that in what I
think is a more appropriate way. It adds a type parameter to
DiscardableMemory::CreateLockedMemory that can be used by clients who want full
control over the type (ie. unit tests). DiscardableMemory::SetDefaultType() in
combination with CreateLockedMemory(DiscardableMemory::GetDefaultType()), can be
used by clients who want set-once-at-startup functionality. Default type is no
longer initialized to a preferred value and ::SetDefaultType() is instead
required to be called prior to ::GetDefaultType(). This allowed me to make the
code both thread-safe and enforce set-once-at-startup usage.
Not seen that paper before but I've made the same claim in the past when
reviewing code some I'm glad to now have this paper to back it up. Thanks!

[willchan no longer on Chromium, 2013/12/26 18:25:54]

Are there any valid reasons for non-test clients to want full control over the
type? I think it'd be nice for most users not to have to pass in the default
type...WDYT about CreateLockedMemory() and
CreateLockedMemoryWithType[ForTests]()?

[reveman, 2013/12/26 21:53:35]

Not at this point.  
Done. I also brought back the default preferred type as I think it would be
weird to require the type to be set before CreateLockedMemory() is called.

The function that needs a platform specific implementation is now
CreateLockedMemoryWithType. CreateLockedMemory is just a convenience function
for "CreateLockedMemoryWithType(GetPreferredType(), size)"

+}
+
+// static
+DiscardableMemoryType DiscardableMemory::GetType() {
+  return g_discardable_memory_type.Get().value;
+}
+
+}  // namespace base