Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
Proposal: use MOZ_LOG instead of NS_WARNING for spammy warnings about huge nscoord values
54 views
Skip to first unread message
Daniel Holbert
Jun 4, 2015, 2:34:03 PM6/4/15
to dev-tec...@lists.mozilla.org
Per this thread...
https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.platform/Jw_4xdUTNnY
...some of the top culprits for NS_WARNING-spam are layout warnings for
huge/unconstrained nscoord math.*
I just talked with jet about this, and he suggested we use MOZ_LOG with
an appropriate log-level, instead of NS_WARNING.
I think this is a good idea. That way, we layout hackers can configure
our environments such that we always see these warnings (if we want to);
meanwhile, our test-runners will have less spam in their logs, and other
developers won't have to see these warnings gunking up their terminals
needlessly, either.
(The downside is, when you're diagnosing a test failure on treeherder,
you might benefit from seeing these warnings; and you can't turn them on
retroactively. However, given the spamminess level of these warnings,
and given that they're not for particularly flaky conditions [we're
either doing math with these values or we arent'], I'm not sure they're
really all that useful in this use case.)
Thoughts?
~Daniel
* specifically, the nscoord-related warnings that erahm reported as
being in the top 40 are:
{
65959 [NNNNN] WARNING: Overflowed nscoord_MAX in conversion to nscoord
width: file ../../dist/include/nsRect.h, line 83
9201 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'psd->mIEnd != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsLineLayout.cpp, line 884
9155 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'psd->mIEnd != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsLineLayout.cpp, line 3058
9130 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'aISize != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp,
line 160
4051 [NNNNN] WARNING: have unconstrained inline-size; this should only
result from very large sizes, not attempts at intrinsic inline-size
calculation: '(mFrameType == NS_CSS_FRAME_TYPE_INLINE &&
!frame->IsFrameOfType(nsIFrame::eReplaced)) || type ==
nsGkAtoms::textFrame || ComputedISize() != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsHTMLReflowState.cpp, line 448
4050 [NNNNN] WARNING: have unconstrained inline-size; this should only
result from very large sizes, not attempts at intrinsic inline-size
calculation: 'AvailableISize() != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsHTMLReflowState.cpp, line 360
3897 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'NS_UNCONSTRAINEDSIZE != aReflowState.ComputedISize()', file
glayout/generic/nsBlockReflowState.cpp, line 118
3892 [NNNNN] WARNING: have unconstrained inline-size; this should only
result from very large sizes, not attempts at intrinsic inline-size
calculation: 'NS_UNCONSTRAINEDSIZE != computedISizeCBWM &&
NS_UNCONSTRAINEDSIZE != availISizeCBWM', file
glayout/generic/nsHTMLReflowState.cpp, line 2398
}
https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.platform/Jw_4xdUTNnY
...some of the top culprits for NS_WARNING-spam are layout warnings for
huge/unconstrained nscoord math.*
I just talked with jet about this, and he suggested we use MOZ_LOG with
an appropriate log-level, instead of NS_WARNING.
I think this is a good idea. That way, we layout hackers can configure
our environments such that we always see these warnings (if we want to);
meanwhile, our test-runners will have less spam in their logs, and other
developers won't have to see these warnings gunking up their terminals
needlessly, either.
(The downside is, when you're diagnosing a test failure on treeherder,
you might benefit from seeing these warnings; and you can't turn them on
retroactively. However, given the spamminess level of these warnings,
and given that they're not for particularly flaky conditions [we're
either doing math with these values or we arent'], I'm not sure they're
really all that useful in this use case.)
Thoughts?
~Daniel
* specifically, the nscoord-related warnings that erahm reported as
being in the top 40 are:
{
65959 [NNNNN] WARNING: Overflowed nscoord_MAX in conversion to nscoord
width: file ../../dist/include/nsRect.h, line 83
9201 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'psd->mIEnd != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsLineLayout.cpp, line 884
9155 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'psd->mIEnd != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsLineLayout.cpp, line 3058
9130 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'aISize != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp,
line 160
4051 [NNNNN] WARNING: have unconstrained inline-size; this should only
result from very large sizes, not attempts at intrinsic inline-size
calculation: '(mFrameType == NS_CSS_FRAME_TYPE_INLINE &&
!frame->IsFrameOfType(nsIFrame::eReplaced)) || type ==
nsGkAtoms::textFrame || ComputedISize() != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsHTMLReflowState.cpp, line 448
4050 [NNNNN] WARNING: have unconstrained inline-size; this should only
result from very large sizes, not attempts at intrinsic inline-size
calculation: 'AvailableISize() != NS_UNCONSTRAINEDSIZE', file
glayout/generic/nsHTMLReflowState.cpp, line 360
3897 [NNNNN] WARNING: have unconstrained width; this should only result
from very large sizes, not attempts at intrinsic width calculation:
'NS_UNCONSTRAINEDSIZE != aReflowState.ComputedISize()', file
glayout/generic/nsBlockReflowState.cpp, line 118
3892 [NNNNN] WARNING: have unconstrained inline-size; this should only
result from very large sizes, not attempts at intrinsic inline-size
calculation: 'NS_UNCONSTRAINEDSIZE != computedISizeCBWM &&
NS_UNCONSTRAINEDSIZE != availISizeCBWM', file
glayout/generic/nsHTMLReflowState.cpp, line 2398
}
Jet Villegas
Jun 10, 2015, 5:12:18 AM6/10/15
to Daniel Holbert, dev-tech-layout
I didn't see any arguments against this change. Since this is the #1
ignored warning in the Gecko tree, with thousands of logs per test run,
let's silence it except for those actively debugging Layout size bugs.
Thanks,
--Jet
On Thu, Jun 4, 2015 at 11:33 AM, Daniel Holbert <dhol...@mozilla.com>
wrote:
> _______________________________________________
> dev-tech-layout mailing list
> dev-tec...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-layout
>
ignored warning in the Gecko tree, with thousands of logs per test run,
let's silence it except for those actively debugging Layout size bugs.
Thanks,
--Jet
On Thu, Jun 4, 2015 at 11:33 AM, Daniel Holbert <dhol...@mozilla.com>
wrote:
> dev-tech-layout mailing list
> dev-tec...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-layout
>
Daniel Holbert
Jun 10, 2015, 2:54:36 PM6/10/15
to j...@mozilla.com, dev-tech-layout
On 06/10/2015 02:12 AM, Jet Villegas wrote:
> I didn't see any arguments against this change. Since this is the #1
> ignored warning in the Gecko tree, with thousands of logs per test run,
> let's silence it except for those actively debugging Layout size bugs.
erahm filed https://bugzilla.mozilla.org/show_bug.cgi?id=1171528 on
> I didn't see any arguments against this change. Since this is the #1
> ignored warning in the Gecko tree, with thousands of logs per test run,
> let's silence it except for those actively debugging Layout size bugs.
suppressing the #1 warning (about nscoord overflow in a saturating math
function within nsRect.h).
While reviewing, I realized that this is really a straggling remnant of
a family of warnings that we removed long ago, in bug 943448; so I
recommended that he just remove this straggling one as well.
I expect that should be landing soon.
~Daniel
er...@mozilla.com
Jun 10, 2015, 6:48:20 PM6/10/15
to
It looks like layout/generic/crashtests/421671.html is responsible for almost all of the other top layout warnings. I wonder if we just want to remove these as well.
42535 - file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/421671.html
2 - No outer window available!: file /dom/base/nsGlobalWindow.cpp, line 3915
3 - have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'NS_UNCONSTRAINEDSIZE != aAvailSpace.width', file /layout/tables/nsTableRowFrame.cpp, line 50
3 - cell content 0x9b9a3190 has large inline size 1073741824
3839 - have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'NS_UNCONSTRAINEDSIZE != aReflowState.ComputedISize()', file /layout/generic/nsBlockReflowState.cpp, line 118
3848 - have unconstrained inline-size; this should only result from very large sizes, not attempts at intrinsic inline-size calculation: 'NS_UNCONSTRAINEDSIZE != computedISizeCBWM && NS_UNCONSTRAINEDSIZE != availISizeCBWM', file /layout/generic/nsHTMLReflowState.cpp, line 2450
3866 - have unconstrained inline-size; this should only result from very large sizes, not attempts at intrinsic inline-size calculation: 'AvailableISize() != NS_UNCONSTRAINEDSIZE', file /layout/generic/nsHTMLReflowState.cpp, line 362
3866 - have unconstrained inline-size; this should only result from very large sizes, not attempts at intrinsic inline-size calculation: '(mFrameType == NS_CSS_FRAME_TYPE_INLINE && !frame->IsFrameOfType(nsIFrame::eReplaced)) || type == nsGkAtoms::textFrame || ComputedISize() != NS_UNCONSTRAINEDSIZE', file /layout/generic/nsHTMLReflowState.cpp, line 454
9036 - have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'psd->mIEnd != NS_UNCONSTRAINEDSIZE', file /layout/generic/nsLineLayout.cpp, line 884
9036 - have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'psd->mIEnd != NS_UNCONSTRAINEDSIZE', file /layout/generic/nsLineLayout.cpp, line 3058
9036 - have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'aISize != NS_UNCONSTRAINEDSIZE', file /layout/generic/nsLineLayout.cpp, line 160
Daniel Holbert
Jun 11, 2015, 1:27:11 PM6/11/15
to er...@mozilla.com, dev-tec...@lists.mozilla.org
Great! (erahm filed bug 1173858 on the other main culprits here, too.)
Thanks for jumping on these!
> It looks like layout/generic/crashtests/421671.html is responsible for almost all of the other top layout warnings.
Yeah -- it uses multicol layout + ends up producing a huge sizes from
the insanely-large number of columns, it looks like.
Multicol layout can require multiple reflows to establish the optimum
column height (and here we have 99999999 columns = 99999999 things being
reflowed each time, I think). And the huge sizes produced by that
column-count will mean we trip lots of warning about working with values
near NS_UNCONSTRAINEDSIZE when we don't expect to see that sentinel-value.
So, we trip a few warnings, and we trip them manymany times. This is
just a pathological testcase (unsurprising for a crashtest).
> I wonder if we just want to remove these as well.
I don't think so; I think we should just make them opt-in (ideally at
runtime with MOZ_LOG). These warnings can be quite useful when
debugging reflow issues (with normal, non-99999999-columned content).
More thoughts on this in
https://bugzilla.mozilla.org/show_bug.cgi?id=1173858#c3
Thanks for jumping on these!
> It looks like layout/generic/crashtests/421671.html is responsible for almost all of the other top layout warnings.
the insanely-large number of columns, it looks like.
Multicol layout can require multiple reflows to establish the optimum
column height (and here we have 99999999 columns = 99999999 things being
reflowed each time, I think). And the huge sizes produced by that
column-count will mean we trip lots of warning about working with values
near NS_UNCONSTRAINEDSIZE when we don't expect to see that sentinel-value.
So, we trip a few warnings, and we trip them manymany times. This is
just a pathological testcase (unsurprising for a crashtest).
> I wonder if we just want to remove these as well.
runtime with MOZ_LOG). These warnings can be quite useful when
debugging reflow issues (with normal, non-99999999-columned content).
More thoughts on this in
https://bugzilla.mozilla.org/show_bug.cgi?id=1173858#c3
Daniel Holbert
Jul 29, 2015, 6:37:38 PM7/29/15
to er...@mozilla.com, dev-tec...@lists.mozilla.org
Closing the loop here -- erahm added LAYOUT_WARNING /
LAYOUT_WARN_IF_FALSE macros, which are backed by some MOZ_LOG stuff. I
posted a PSA about the new macros here:
https://groups.google.com/d/msg/mozilla.dev.tech.layout/xsjIrxt9NLM/aEQtVYaRBQAJ
LAYOUT_WARN_IF_FALSE macros, which are backed by some MOZ_LOG stuff. I
posted a PSA about the new macros here:
https://groups.google.com/d/msg/mozilla.dev.tech.layout/xsjIrxt9NLM/aEQtVYaRBQAJ
0 new messages