Oilpan: enable eager tracing.

Source/platform/heap/Visitor.h

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 169 matching lines @@
 #if ENABLE(ASSERT)
     static void checkGCInfo(Visitor* visitor, const T* t)
     {
         DefaultTraceTrait<T>::checkGCInfo(visitor, t);
     }
 #endif
 };
 
 template<typename T> class TraceTrait<const T> : public TraceTrait<T> { };
 
-// If ENABLE_EAGER_TRACING_BY_DEFAULT is set to 1, GCed objects will
-// be eagerly traced by default. A class type can opt out by declaring
-// a TraceEagerlyTrait<> specialization, mapping the value to 'false'
-// (see the WILL_NOT_BE_EAGERLY_TRACED() macro below.)
-#define ENABLE_EAGER_TRACING_BY_DEFAULT 0
+// If MARKER_EAGER_TRACING is set to 1, a marker thread is allowed
+// to directly invoke the trace() method of not-as-yet marked objects upon
+// marking. If it is set to 0, the |trace()| callback for an object will
+// be pushed onto an explicit mark stack, which the marker proceeds to
+// iteratively pop and invoke. The eager scheme enables inlining of a trace()
+// method inside another, the latter keeps system call stack usage bounded
+// and under explicit control.
+//
+// If eager tracing leads to excessively deep |trace()| call chains (and
+// the system stack usage that this brings), the marker implementation will
+// switch to using an explicit mark stack. Recursive and deep object graphs
+// are uncommon for Blink objects.
+//
+// A class type can opt out of eager tracing by declaring a TraceEagerlyTrait<>
+// specialization, mapping the trait's |value| to |false| (see the
+// WILL_NOT_BE_EAGERLY_TRACED() macros below.) For Blink, this is done for
+// the small set of GCed classes that are directly recursive.
+#define MARKER_EAGER_TRACING 1
 
-// DISABLE_ALL_EAGER_TRACING provides the "kill switch" for eager
-// tracing; setting it to 1 will disable the use of eager tracing
-// entirely. That is, eager tracing is disabled even if traits have
-// been declared.
-#define DISABLE_ALL_EAGER_TRACING 0
-
-// If TraceEagerlyTrait<T>::value is true, then the marker thread should
-// invoke trace() on not-yet-marked objects deriving from class T right
-// away, and not queue their trace callbacks on its marker stack.
+// The TraceEagerlyTrait<T> trait controls whether or not a class
+// (and its subclasses) should be eagerly traced or not.
 //
-// Specific template specializations of TraceEagerlyTrait<T> can be used
-// to declare that eager tracing should always be used when tracing over
-// GCed objects with class type T. If the trait's boolean 'value' is
-// mapped to 'true' that is; declare it as 'false' to disable eager tracing.
+// If |TraceEagerlyTrait<T>::value| is |true|, then the marker thread
+// should invoke |trace()| on not-yet-marked objects deriving from class T
+// right away, and not queue their trace callbacks on its marker stack,
+// which it will do if |value| is |false|.
 //
 // The trait can be declared to enable/disable eager tracing for a class T
-// and any of its subclasses, or just to the class T (but none of its subclasses.)
+// and any of its subclasses, or just to the class T, but none of its
+// subclasses.
 //
 template<typename T, typename Enabled = void>
 class TraceEagerlyTrait {
 public:
-    static const bool value = ENABLE_EAGER_TRACING_BY_DEFAULT;
+    static const bool value = MARKER_EAGER_TRACING;
 };
 
 #define WILL_BE_EAGERLY_TRACED(TYPE)                                                        \

[haraken, 2014/12/09 15:47:24]

This macro is no longer needed probably?

[sof, 2014/12/09 21:55:37]

No particular harm in having it around, but now unused on trunk. Hence removed.

-template<typename U>                                                                        \
-class TraceEagerlyTrait<U, typename WTF::EnableIf<WTF::IsSubclass<U, TYPE>::value>::Type> { \
+template<typename T>                                                                        \
+class TraceEagerlyTrait<T, typename WTF::EnableIf<WTF::IsSubclass<T, TYPE>::value>::Type> { \
 public:                                                                                     \
     static const bool value = true;                                                         \
 }
 
 #define WILL_NOT_BE_EAGERLY_TRACED(TYPE)                                                    \

[haraken, 2014/12/09 15:47:24]

Ditto.

[sof, 2014/12/09 21:55:37]

Kept, as it serves a purpose.

-template<typename U>                                                                        \
-class TraceEagerlyTrait<U, typename WTF::EnableIf<WTF::IsSubclass<U, TYPE>::value>::Type> { \
+template<typename T>                                                                        \
+class TraceEagerlyTrait<T, typename WTF::EnableIf<WTF::IsSubclass<T, TYPE>::value>::Type> { \
 public:                                                                                     \
     static const bool value = false;                                                        \
 }
 
-// Limit eager tracing to only apply to TYPE (but not any of its subclasses.)
+// Enable eager tracing for TYPE, but not any of its subclasses.
 #define WILL_BE_EAGERLY_TRACED_CLASS(TYPE)       \

[haraken, 2014/12/09 15:47:24]

Ditto. If it's unused, we might want to drop this.

[sof, 2014/12/09 21:55:37]

Retired also.

 template<>                                       \
 class TraceEagerlyTrait<TYPE> {                  \
 public:                                          \
     static const bool value = true;              \
 }
 
+// Disable eager tracing for TYPE, but not any of its subclasses.
 #define WILL_NOT_BE_EAGERLY_TRACED_CLASS(TYPE)   \
 template<>                                       \
 class TraceEagerlyTrait<TYPE> {                  \
 public:                                          \
     static const bool value = false;             \
 }
 
-// Set to 1 if you want collections to be eagerly traced regardless
-// of whether the elements are eagerly traceable or not.
-#define ENABLE_EAGER_HEAP_COLLECTION_TRACING ENABLE_EAGER_TRACING_BY_DEFAULT
-
-#if ENABLE_EAGER_HEAP_COLLECTION_TRACING
-#define IS_EAGERLY_TRACED_HEAP_COLLECTION(Type) true
-#else
-#define IS_EAGERLY_TRACED_HEAP_COLLECTION(Type) TraceEagerlyTrait<Type>::value
-#endif
-
 template<typename Collection>
 struct OffHeapCollectionTraceTrait;
 
 template<typename T>
 struct ObjectAliveTrait {
     static bool isHeapObjectAlive(Visitor*, T*);
 };
 
 // Visitor is used to traverse the Blink object graph. Used for the
 // marking phase of the mark-sweep garbage collector.
@@ skipping 236 matching lines @@
 
 #if ENABLE(GC_PROFILE_MARKING)
     void setHostInfo(void* object, const String& name)
     {
         m_hostObject = object;
         m_hostName = name;
     }
 #endif
 
     inline bool canTraceEagerly() const { return m_traceDepth < kMaxEagerTraceDepth; }
     inline void incrementTraceDepth() { m_traceDepth++; }

[Erik Corry, 2014/12/09 15:49:02]

Perhaps not in this CL, but it would be great to test the stack pointer hack
from
https://groups.google.com/a/chromium.org/d/msg/oilpan-reviews/RiWQ8Gy_dBA/PRv...
to avoid the increment and decrement.

[sof, 2014/12/09 15:56:40]

Numbers from 

 
https://docs.google.com/spreadsheets/d/1ff-QKXNne6TaWbbAqmmoDQdAeEay2nDDgpYNE...

suggests there is not much of a difference between the two.

     inline void decrementTraceDepth() { ASSERT(m_traceDepth > 0); m_traceDepth--; }
 
 protected:
     Visitor()
         : m_traceDepth(0)
     {
     }
 
     virtual void registerWeakCell(void**, WeakPointerCallback) = 0;
 #if ENABLE(GC_PROFILE_MARKING)
@@ skipping 60 matching lines @@
 public:
     static void mark(Visitor* visitor, const T* t)
     {
         // Default mark method of the trait just calls the two-argument mark
         // method on the visitor. The second argument is the static trace method
         // of the trait, which by default calls the instance method
         // trace(Visitor*) on the object.
         //
         // If the trait allows it, invoke the trace callback right here on the
         // not-yet-marked object.
-        if (!DISABLE_ALL_EAGER_TRACING && TraceEagerlyTrait<T>::value) {
+        if (TraceEagerlyTrait<T>::value) {
             // Protect against too deep trace call chains, and the
             // unbounded system stack usage they can bring about.
             //
             // Assert against deep stacks so as to flush them out,
             // but test and appropriately handle them should they occur
             // in release builds.
             ASSERT(visitor->canTraceEagerly());
             if (LIKELY(visitor->canTraceEagerly())) {
                 if (visitor->ensureMarked(t)) {
                     visitor->incrementTraceDepth();
@@ skipping 90 matching lines @@
     virtual bool isHeapObjectAlive(Visitor*) const = 0;
     virtual void trace(Visitor*) { }
 };
 
 #define USING_GARBAGE_COLLECTED_MIXIN(TYPE) \
 public: \
     virtual void adjustAndMark(blink::Visitor* visitor) const override \
     { \
         typedef WTF::IsSubclassOfTemplate<typename WTF::RemoveConst<TYPE>::Type, blink::GarbageCollected> IsSubclassOfGarbageCollected; \
         COMPILE_ASSERT(IsSubclassOfGarbageCollected::value, OnlyGarbageCollectedObjectsCanHaveGarbageCollectedMixins);                  \
-        if (!DISABLE_ALL_EAGER_TRACING && TraceEagerlyTrait<TYPE>::value) {             \
+        if (TraceEagerlyTrait<TYPE>::value) {                                           \
             if (visitor->ensureMarked(static_cast<const TYPE*>(this)))                  \
                 TraceTrait<TYPE>::trace(visitor, const_cast<TYPE*>(this));              \
             return;                                                                     \
         }                                                                               \
         visitor->mark(static_cast<const TYPE*>(this), &blink::TraceTrait<TYPE>::trace); \
     } \
     virtual bool isHeapObjectAlive(blink::Visitor* visitor) const override              \
     { \
         return visitor->isAlive(this); \
     } \
@@ skipping 50 matching lines @@
 struct GCInfoTrait {
     static const GCInfo* get()
     {
         return GCInfoAtBase<typename GetGarbageCollectedBase<T>::type>::get();
     }
 };
 
 }
 
 #endif