cc: Adding creation location to debug BeginFrameArgs objects.

cc/output/begin_frame_args.h

Index: cc/output/begin_frame_args.h
diff --git a/cc/output/begin_frame_args.h b/cc/output/begin_frame_args.h
index c35a5005695faa802e0737e5bc2ba8b9661dd742..1ccbbb8147526ff6c5563eeb9c3844483d4defcf 100644
--- a/cc/output/begin_frame_args.h
+++ b/cc/output/begin_frame_args.h
@@ -5,6 +5,7 @@
 #ifndef CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 #define CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 
+#include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "base/values.h"
@@ -17,6 +18,22 @@ class TracedValue;
 }
 }
 
+/**
+ * In debug builds we trace the creation origin of BeginFrameArgs objects. We
+ * reuse the tracked_objects::Location system to do that.
+ *
+ * However, in release builds we don't want this as it doubles the size of the
+ * BeginFrameArgs object. As well it adds a number of largish strings to the
+ * binary. Despite the argument being unused, most compilers are unable to
+ * optimise it away even when unused. Instead we use the BEGINFRAME_FROM_HERE
+ * macro to prevent the data even getting referenced.
+ */
+#ifdef NDEBUG
+#define BEGINFRAME_FROM_HERE nullptr
+#else
+#define BEGINFRAME_FROM_HERE (new FROM_HERE)

[Sami, 2014/11/20 11:36:13]

Do we need to allocate FROM_HEREs on the heap? They can usually be passed around
by value. I don't think creating a dummy location in release mode would add all
that much code if you consider that there aren't that many places that create
BeginFrames in non-test code.

Which compilers did you check that the strings cannot be pruned? At least gcc
4.8.2 with -flto was able to drop the strings even when they were passed between
compilation units.

[danakj, 2014/11/20 16:02:20]

I thought that too, but then you can't avoid allocating one in release builds.

[mithro-old, 2014/11/21 05:57:46]

I originally thought that too but with the typedef based solution it does work.

[mithro-old, 2014/11/21 05:57:46]

Actually, with the typedef approach I just added, these don't have to be heap
allocated anymore. See the updated change.
I'm using clang shipping with chrome and the binary definitely continued to
contain the strings when I checked with "strings".

+#endif
+
 namespace cc {
 
 struct CC_EXPORT BeginFrameArgs {
@@ -26,21 +43,26 @@ struct CC_EXPORT BeginFrameArgs {
     SYNCHRONOUS,
     MISSED,
   };
+  static const char* TypeToString(BeginFrameArgsType type);
 
   // Creates an invalid set of values.
   BeginFrameArgs();
 
+#ifdef NDEBUG
+  typedef const void* CreationLocation;
+#else
+  typedef const tracked_objects::Location* CreationLocation;
+  tracked_objects::Location created_by;

[Sami, 2014/11/20 11:36:14]

bikeshed: created_from?

[mithro-old, 2014/11/21 05:57:46]

Done.

+#endif
+
   // You should be able to find all instances where a BeginFrame has been
   // created by searching for "BeginFrameArgs::Create".
-  static BeginFrameArgs Create(base::TimeTicks frame_time,
+  // The location argument should **always** be BEGINFRAME_FROM_HERE macro.
+  static BeginFrameArgs Create(CreationLocation location,
+                               base::TimeTicks frame_time,
                                base::TimeTicks deadline,
-                               base::TimeDelta interval);
-  static BeginFrameArgs CreateTyped(base::TimeTicks frame_time,
-                                    base::TimeTicks deadline,
-                                    base::TimeDelta interval,
-                                    BeginFrameArgsType type);
-  static BeginFrameArgs CreateForSynchronousCompositor(
-      base::TimeTicks now = base::TimeTicks());
+                               base::TimeDelta interval,
+                               BeginFrameArgsType type);
 
   // This is the default delta that will be used to adjust the deadline when
   // proper draw-time estimations are not yet available.