Bug 1596973 - Improve FlexContainer's logging facility. r=dholbert
authorTing-Yu Lin <tlin@mozilla.com>
Sat, 16 Nov 2019 00:57:50 +0000
changeset 502324 e6bc09bed1cf8e008b4b67a562154d53251812b8
parent 502323 05f4ac2d6dd6beb54482ce138d36cf3226f2464c
child 502325 449f5545954f8bde849fcc372b4b87a6338739c1
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1596973
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1596973 - Improve FlexContainer's logging facility. r=dholbert It is a bit awkward to use raw MOZ_LOG directly, so I added FLEX_LOG() to easily add `printf()` style logs. "\n" is not need because MOZ_LOG() already appends one. I hope it is OK to change the log module name to "FlexContainer". It's shorter to type either by using `MOZ_LOG=FlexContainer:debug ./mach run` or by setting `logging.FlexContainer=debug` in [runprefs] section in `~/.mozbuild/machrc` Differential Revision: https://phabricator.services.mozilla.com/D53299
layout/generic/nsFlexContainerFrame.cpp
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -35,17 +35,19 @@ using namespace mozilla::layout;
 typedef nsFlexContainerFrame::FlexItem FlexItem;
 typedef nsFlexContainerFrame::FlexLine FlexLine;
 typedef nsFlexContainerFrame::FlexboxAxisTracker FlexboxAxisTracker;
 typedef nsFlexContainerFrame::StrutInfo StrutInfo;
 typedef nsFlexContainerFrame::CachedMeasuringReflowResult
     CachedMeasuringReflowResult;
 typedef nsLayoutUtils::IntrinsicISizeType IntrinsicISizeType;
 
-static mozilla::LazyLogModule gFlexContainerLog("nsFlexContainerFrame");
+static mozilla::LazyLogModule gFlexContainerLog("FlexContainer");
+#define FLEX_LOG(...) \
+  MOZ_LOG(gFlexContainerLog, LogLevel::Debug, (__VA_ARGS__));
 
 // XXXdholbert Some of this helper-stuff should be separated out into a general
 // "main/cross-axis utils" header, shared by grid & flexbox?
 // (Particularly when grid gets support for align-*/justify-* properties.)
 
 // Helper enums
 // ============
 
@@ -1792,23 +1794,20 @@ const CachedMeasuringReflowResult&
 nsFlexContainerFrame::MeasureAscentAndBSizeForFlexItem(
     FlexItem& aItem, nsPresContext* aPresContext,
     ReflowInput& aChildReflowInput) {
   if (const auto* cachedResult =
           aItem.Frame()->GetProperty(CachedFlexMeasuringReflow())) {
     if (cachedResult->IsValidFor(aChildReflowInput)) {
       return *cachedResult;
     }
-    MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-            ("[perf] MeasureAscentAndBSizeForFlexItem rejected "
-             "cached value\n"));
+    FLEX_LOG("[perf] MeasureAscentAndBSizeForFlexItem rejected cached value");
   } else {
-    MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-            ("[perf] MeasureAscentAndBSizeForFlexItem didn't have a "
-             "cached value\n"));
+    FLEX_LOG(
+        "[perf] MeasureAscentAndBSizeForFlexItem didn't have a cached value");
   }
 
   ReflowOutput childDesiredSize(aChildReflowInput);
   nsReflowStatus childReflowStatus;
 
   const ReflowChildFlags flags = ReflowChildFlags::NoMoveFrame;
   ReflowChild(aItem.Frame(), aPresContext, childDesiredSize, aChildReflowInput,
               0, 0, flags, childReflowStatus);
@@ -2634,17 +2633,17 @@ void FlexLine::FreezeOrRestoreEachFlexib
         item->ClearViolationFlags();
       }
     }
   }
 }
 
 void FlexLine::ResolveFlexibleLengths(nscoord aFlexContainerMainSize,
                                       ComputedFlexLineInfo* aLineInfo) {
-  MOZ_LOG(gFlexContainerLog, LogLevel::Debug, ("ResolveFlexibleLengths\n"));
+  FLEX_LOG("ResolveFlexibleLengths");
 
   // Before we start resolving sizes: if we have an aLineInfo structure to fill
   // out, we inform it of each item's base size, and we initialize the "delta"
   // for each item to 0. (And if the flex algorithm wants to grow or shrink the
   // item, we'll update this delta further down.)
   if (aLineInfo) {
     uint32_t itemIndex = 0;
     for (FlexItem* item = mItems.getFirst(); item;
@@ -2700,18 +2699,17 @@ void FlexLine::ResolveFlexibleLengths(ns
     nscoord availableFreeSpace = spaceAvailableForFlexItemsContentBoxes;
     for (FlexItem* item = mItems.getFirst(); item; item = item->getNext()) {
       if (!item->IsFrozen()) {
         item->SetMainSize(item->GetFlexBaseSize());
       }
       availableFreeSpace -= item->GetMainSize();
     }
 
-    MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-            (" available free space = %d\n", availableFreeSpace));
+    FLEX_LOG(" available free space = %d", availableFreeSpace);
 
     // The sign of our free space should agree with the type of flexing
     // (grow/shrink) that we're doing (except if we've had integer overflow;
     // then, all bets are off). Any disagreement should've made us use the
     // other type of flexing, or should've been resolved in FreezeItemsEarly.
     // XXXdholbert If & when bug 765861 is fixed, we should upgrade this
     // assertion to be fatal except in documents with enormous lengths.
     NS_ASSERTION((isUsingFlexGrow && availableFreeSpace >= 0) ||
@@ -2812,18 +2810,17 @@ void FlexLine::ResolveFlexibleLengths(ns
             availableFreeSpace = std::min(availableFreeSpace,
                                           totalDesiredPortionOfOrigFreeSpace);
           } else {
             availableFreeSpace = std::max(availableFreeSpace,
                                           totalDesiredPortionOfOrigFreeSpace);
           }
         }
 
-        MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-                (" Distributing available space:"));
+        FLEX_LOG(" Distributing available space:");
         // Since this loop only operates on unfrozen flex items, we can break as
         // soon as we have seen all of them.
         numUnfrozenItemsToBeSeen = mNumItems - mNumFrozenItems;
 
         // NOTE: It's important that we traverse our items in *reverse* order
         // here, for correct width distribution according to the items'
         // "ShareOfWeightSoFar" progressively-calculated values.
         for (FlexItem* item = mItems.getLast(); numUnfrozenItemsToBeSeen > 0;
@@ -2858,19 +2855,18 @@ void FlexLine::ResolveFlexibleLengths(ns
               sizeDelta = NSToCoordRound(availableFreeSpace /
                                          float(numItemsWithLargestWeight));
               numItemsWithLargestWeight--;
             }
 
             availableFreeSpace -= sizeDelta;
 
             item->SetMainSize(item->GetMainSize() + sizeDelta);
-            MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-                    ("  child %p receives %d, for a total of %d\n", item,
-                     sizeDelta, item->GetMainSize()));
+            FLEX_LOG("  child %p receives %d, for a total of %d", item,
+                     sizeDelta, item->GetMainSize());
           }
         }
 
         // If we have an aLineInfo structure to fill out, capture any
         // size changes that may have occurred in the previous loop.
         // We don't do this inside the previous loop, because we don't
         // want to burden layout when aLineInfo is null.
         if (aLineInfo) {
@@ -2894,17 +2890,17 @@ void FlexLine::ResolveFlexibleLengths(ns
             }
           }
         }
       }
     }
 
     // Fix min/max violations:
     nscoord totalViolation = 0;  // keeps track of adjustments for min/max
-    MOZ_LOG(gFlexContainerLog, LogLevel::Debug, (" Checking for violations:"));
+    FLEX_LOG(" Checking for violations:");
 
     // Since this loop only operates on unfrozen flex items, we can break as
     // soon as we have seen all of them.
     uint32_t numUnfrozenItemsToBeSeen = mNumItems - mNumFrozenItems;
     for (FlexItem* item = mItems.getFirst(); numUnfrozenItemsToBeSeen > 0;
          item = item->getNext()) {
       MOZ_ASSERT(item, "numUnfrozenItemsToBeSeen says items remain to be seen");
       if (!item->IsFrozen()) {
@@ -2922,18 +2918,17 @@ void FlexLine::ResolveFlexibleLengths(ns
           item->SetHadMaxViolation();
         }
       }
     }
 
     FreezeOrRestoreEachFlexibleSize(totalViolation,
                                     iterationCounter + 1 == mNumItems);
 
-    MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-            (" Total violation: %d\n", totalViolation));
+    FLEX_LOG(" Total violation: %d", totalViolation);
 
     if (mNumFrozenItems == mNumItems) {
       break;
     }
 
     MOZ_ASSERT(totalViolation != 0,
                "Zero violation should've made us freeze all items & break");
   }
@@ -4315,18 +4310,17 @@ void nsFlexContainerFrame::DidReflow(nsP
 void nsFlexContainerFrame::Reflow(nsPresContext* aPresContext,
                                   ReflowOutput& aDesiredSize,
                                   const ReflowInput& aReflowInput,
                                   nsReflowStatus& aStatus) {
   MarkInReflow();
   DO_GLOBAL_REFLOW_COUNT("nsFlexContainerFrame");
   DISPLAY_REFLOW(aPresContext, this, aReflowInput, aDesiredSize, aStatus);
   MOZ_ASSERT(aStatus.IsEmpty(), "Caller should pass a fresh reflow status!");
-  MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-          ("Reflow() for nsFlexContainerFrame %p\n", this));
+  FLEX_LOG("Reflow() for nsFlexContainerFrame %p", this);
 
   if (IsFrameTreeTooDeep(aReflowInput, aDesiredSize, aStatus)) {
     return;
   }
 
   // We (and our children) can only depend on our ancestor's bsize if we have
   // a percent-bsize, or if we're positioned and we have "block-start" and
   // "block-end" set and have block-size:auto.  (There are actually other cases,
@@ -4989,19 +4983,19 @@ void nsFlexContainerFrame::DoFlexLayout(
             // it's now "definite"). Let's just make sure it's at the right
             // position.
             itemNeedsReflow = false;
             MoveFlexItemToFinalPosition(aReflowInput, *item, framePos,
                                         containerSize);
           }
         }
         if (itemNeedsReflow) {
-          MOZ_LOG(gFlexContainerLog, LogLevel::Debug,
-                  ("[perf] Flex item needed both a measuring reflow and "
-                   "a final reflow\n"));
+          FLEX_LOG(
+              "[perf] Flex item needed both a measuring reflow and a final "
+              "reflow");
         }
       }
       if (itemNeedsReflow) {
         ReflowFlexItem(aPresContext, aAxisTracker, aReflowInput, *item,
                        framePos, containerSize, aHasLineClampEllipsis);
       }
 
       // If we didn't perform a final reflow of the item, we still have a