[tracing] Add method to create "weak" global dumps

base/trace_event/memory_allocator_dump.h

 // Copyright 2015 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.
 
 #ifndef BASE_TRACE_EVENT_MEMORY_ALLOCATOR_DUMP_H_
 #define BASE_TRACE_EVENT_MEMORY_ALLOCATOR_DUMP_H_
 
 #include <stdint.h>
 
 #include <string>
 
 #include "base/base_export.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/trace_event/memory_allocator_dump_guid.h"
 #include "base/values.h"
 
 namespace base {
 namespace trace_event {
 
 class MemoryDumpManager;
 class ProcessMemoryDump;
 class TracedValue;
 
 // Data model for user-land memory allocator dumps.
 class BASE_EXPORT MemoryAllocatorDump {
  public:
+  enum Flags {

[petrcermak, 2016/01/13 17:46:14]

I think that this should be called "Flag" (singular, see
https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&l=151)

[ssid, 2016/01/15 17:14:56]

Done.

+    // A weak dump is made invalid unless it is marked strong later, either by

[petrcermak, 2016/01/13 17:46:14]

It could also be 'marked' strong "earlier".

[petrcermak, 2016/01/13 17:46:14]

Please don't use the word "invalid" because we don't use it anywhere else and it
is unclear what it means. You could say something along the lines of the dump
being "ignored", "removed", ...

[ssid, 2016/01/15 17:14:56]

Done.

[ssid, 2016/01/15 17:14:56]

Done.

+    // the same process or at least one process in case of global dumps.
+    // All dumps that own an invalid dump and all its children will be marked

[petrcermak, 2016/01/13 17:46:14]

nit: I'd say "All owners and children of a removed dump will also be removed
transitively"

[ssid, 2016/01/15 17:14:56]

Done.

+    // invalid transitively. Default value is 0.

[petrcermak, 2016/01/13 17:46:14]

Rather than "Default value is 0", say "A dump is strong by default".

[ssid, 2016/01/15 17:14:56]

Done.

+    WEAK = 1 << 0
+  };
+
   // MemoryAllocatorDump is owned by ProcessMemoryDump.
   MemoryAllocatorDump(const std::string& absolute_name,
                       ProcessMemoryDump* process_memory_dump,
                       const MemoryAllocatorDumpGuid& guid);
   MemoryAllocatorDump(const std::string& absolute_name,
                       ProcessMemoryDump* process_memory_dump);
   ~MemoryAllocatorDump();
 
   // Standard attribute |name|s for the AddScalar and AddString() methods.
   static const char kNameSize[];          // To represent allocated space.
@@ skipping 14 matching lines @@
   //    AddScalar("number_of_freelist_entires", kUnitsObjects, 42)
   // - Other informational column (will not be auto-added in the UI)
   //    AddScalarF("kittens_ratio", "ratio", 42.0f)
   void AddScalar(const char* name, const char* units, uint64_t value);
   void AddScalarF(const char* name, const char* units, double value);
   void AddString(const char* name, const char* units, const std::string& value);
 
   // Absolute name, unique within the scope of an entire ProcessMemoryDump.
   const std::string& absolute_name() const { return absolute_name_; }
 
+  // Use Flags to set values.
+  void set_flags(int flags) { flags_ |= flags; }
+  void reset_flags(int flags) { flags_ &= !flags; }

[petrcermak, 2016/01/13 17:46:14]

it this a naming convention? If not, "unset" might be less ambiguous.

[ssid, 2016/01/15 17:14:56]

Done.

+  bool flags() const { return flags_; }

[petrcermak, 2016/01/13 17:46:14]

this shouldn't be a bool...

+
   // Called at trace generation time to populate the TracedValue.
   void AsValueInto(TracedValue* value) const;
 
   // Get the ProcessMemoryDump instance that owns this.
   ProcessMemoryDump* process_memory_dump() const {
     return process_memory_dump_;
   }
 
   // |guid| is an optional global dump identifier, unique across all processes
   // within the scope of a global dump. It is only required when using the
   // graph APIs (see TODO_method_name) to express retention / suballocation or
   // cross process sharing. See crbug.com/492102 for design docs.
   // Subsequent MemoryAllocatorDump(s) with the same |absolute_name| are
   // expected to have the same guid.
   const MemoryAllocatorDumpGuid& guid() const { return guid_; }
 
   TracedValue* attributes_for_testing() const { return attributes_.get(); }
 
  private:
   const std::string absolute_name_;
   ProcessMemoryDump* const process_memory_dump_;  // Not owned (PMD owns this).
   scoped_refptr<TracedValue> attributes_;
   MemoryAllocatorDumpGuid guid_;
+  int flags_;  // See Flags.

[petrcermak, 2016/01/13 17:46:14]

I think that this should be unsigned or Flag (even though you can assign
combinations of flags to it)

[ssid, 2016/01/15 17:14:56]

Hm I made this integer because the traceValue accepts only integer or double.
Not unsigned. Fixed and added a cast there.

 
   // A local buffer for Sprintf conversion on fastpath. Avoids allocating
   // temporary strings on each AddScalar() call.
   std::string string_conversion_buffer_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryAllocatorDump);
 };
 
 }  // namespace trace_event
 }  // namespace base
 
 #endif  // BASE_TRACE_EVENT_MEMORY_ALLOCATOR_DUMP_H_