Closed Bug 1377707 Opened 2 years ago Closed 2 years ago

nsLayoutUtils::LogTestDataForPaint shows up in performance profiles even when not logging

Categories

(Core :: Web Painting, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: mats, Assigned: mats)

References

Details

(Keywords: perf, Whiteboard: [qf])

Attachments

(1 file)

I'm seeing malloc/free churn from this (reversed) stack:

...
mozilla::ScrollFrameHelper::ComputeScrollMetadata(mozilla::layers::Layer*, nsIFrame*, mozilla::ContainerLayerParameters const&, mozilla::DisplayItemClip const*) const
GetScrollFrameFromContent(nsIContent*)
std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&)
std::string::_S_construct_aux_2(unsigned long, char, std::allocator<char> const&)
std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&)
operator new(unsigned long)
je_malloc


I'm guessing it comes from allocating a std::string for "displayport" etc:
http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/layout/base/nsLayoutUtils.cpp#8788

It seems like a general problem with LogTestDataForPaint that we
evaluate arguments for it even when we're not logging.

The code should instead be something like:

if (IsAPZTestLoggingEnabled()) {
  nsLayoutUtils::LogTestDataForPaint(aLayer->Manager(), scrollId, "displayport",
                                     metrics.GetDisplayPort());
}

to make it zero-cost when logging is off.
Blocks: 1369839
Attachment #8882843 - Flags: review?(tnikkel) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/568603e363bb
Ensure that LogTestDataForPaint is not called at all when APZTestLoggingEnabled() is false.  r=tn
https://hg.mozilla.org/mozilla-central/rev/568603e363bb
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1389335
You need to log in before you can comment on or make changes to this bug.