Issue 1485813002: Reland of Fix regression in memory-infra bots due to change in dump order

Issue 1485813002: Reland of Fix regression in memory-infra bots due to change in dump order (Closed)

Created:
5 years ago by ssid

Modified:
4 years, 11 months ago

Reviewers:
Primiano Tucci (use gerrit), dsinclair, picksi1

CC:
chromium-reviews, tracing+reviews_chromium.org, wfh+watch_chromium.org, vmpstr+watch_chromium.org, bashi

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Reland of Fix regression in memory-infra bots due to change in dump order (patchset #1 id:1 of https://codereview.chromium.org/1479003003/ ) Reason for revert: We have seen that the regression is fixed for renderer process, but not browser process. The browser process regression is due to other CLs. https://goo.gl/2I5xkG Original issue's description: > Revert of Fix regression in memory-infra bots due to change in dump order (patchset #1 id:1 of https://codereview.chromium.org/1466293002/ ) > > Reason for revert: > Along with this CL, the memory-infra build was broken > (fixed in https://codereview.chromium.org/1474543003/). But, this > cause much noise in the metrics and did not give a clear idea if the > change fixed the original regression. So, I will revert this again > and re-land it to check if that actually makes a difference. > > Original issue's description: > > Fix regression in memory-infra bots due to change in dump order > > > > crrev.com/1427963002 introduced a new memory-infra thread which runs > > after all dump providers are finished. This measures the process total > > memory usages. This was previously running at the start of a memory > > dump. This means that the memory used while taking memory dump of other > > providers will be included in the totals now and this metric has > > regressed. This CL is a temporary change that reverts this behaviour so > > that the perf bots are happy. Better fix for the problem will follow up > > soon. > > > > BUG=555584 > > > > Committed: https://crrev.com/ef89c2a502dadf4458000278ef4befbd79523bf8 > > Cr-Commit-Position: refs/heads/master@{#361126} > > NOPRESUBMIT=true > NOTREECHECKS=true > BUG=555584 > > Committed: https://crrev.com/6bf41bd974a754ee4e80f77099f4e0bf0bc793fa > Cr-Commit-Position: refs/heads/master@{#361899} TBR=dsinclair@chromium.org,primiano@chromium.org,picksi@chromium.org NOPRESUBMIT=true NOTREECHECKS=true BUG=555584

Patch Set 1 #

Created: 5 years ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+7 lines, -3 lines)			Patch
	M	base/trace_event/memory_dump_manager.cc	View		1 chunk	+5 lines, -2 lines	0 comments	Download
	M	base/trace_event/memory_dump_manager_unittest.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download

Messages

Total messages: 7 (4 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

ssid

Created Reland of Fix regression in memory-infra bots due to change in dump order

5 years ago (2015-11-30 18:30:38 UTC) #1

ssid

Description was changed from ========== Reland of Fix regression in memory-infra bots due to change ...

5 years ago (2015-11-30 18:30:58 UTC) #2

Description was changed from

==========
Reland of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1479003003/ )

Reason for revert:
We have seen that the regression is fixed for renderer process, but not browser
process. The browser process regression is due to other CLs.
https://goo.gl/2I5xkG

Original issue's description:
> Revert of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1466293002/ )
> 
> Reason for revert:
> Along with this CL, the memory-infra build was broken
> (fixed in https://codereview.chromium.org/1474543003/). But, this
> cause much noise in the metrics and did not give a clear idea if the
> change fixed the original regression. So, I will revert this again
> and re-land it to check if that actually makes a difference.
> 
> Original issue's description:
> > Fix regression in memory-infra bots due to change in dump order
> >
> > crrev.com/1427963002 introduced a new memory-infra thread which runs
> > after all dump providers are finished. This measures the process total
> > memory usages. This was previously running at the start of a memory
> > dump. This means that the memory used while taking memory dump of other
> > providers will be included in the totals now and this metric has
> > regressed. This CL is a temporary change that reverts this behaviour so
> > that the perf bots are happy. Better fix for the problem will follow up
> > soon.
> >
> > BUG=555584
> >
> > Committed: https://crrev.com/ef89c2a502dadf4458000278ef4befbd79523bf8
> > Cr-Commit-Position: refs/heads/master@{#361126}
> 
> NOPRESUBMIT=true
> NOTREECHECKS=true
> BUG=555584
> 
> Committed: https://crrev.com/6bf41bd974a754ee4e80f77099f4e0bf0bc793fa
> Cr-Commit-Position: refs/heads/master@{#361899}

TBR=dsinclair@chromium.org,primiano@chromium.org,picksi@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=555584
==========

to

==========
Reland of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1479003003/ )

Reason for revert:
We have seen that the regression is fixed for renderer process, but not browser
process. The browser process regression is due to other CLs.
https://goo.gl/2I5xkG

Original issue's description:
> Revert of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1466293002/ )
> 
> Reason for revert:
> Along with this CL, the memory-infra build was broken
> (fixed in https://codereview.chromium.org/1474543003/). But, this
> cause much noise in the metrics and did not give a clear idea if the
> change fixed the original regression. So, I will revert this again
> and re-land it to check if that actually makes a difference.
> 
> Original issue's description:
> > Fix regression in memory-infra bots due to change in dump order
> >
> > crrev.com/1427963002 introduced a new memory-infra thread which runs
> > after all dump providers are finished. This measures the process total
> > memory usages. This was previously running at the start of a memory
> > dump. This means that the memory used while taking memory dump of other
> > providers will be included in the totals now and this metric has
> > regressed. This CL is a temporary change that reverts this behaviour so
> > that the perf bots are happy. Better fix for the problem will follow up
> > soon.
> >
> > BUG=555584
> >
> > Committed: https://crrev.com/ef89c2a502dadf4458000278ef4befbd79523bf8
> > Cr-Commit-Position: refs/heads/master@{#361126}
> 
> NOPRESUBMIT=true
> NOTREECHECKS=true
> BUG=555584
> 
> Committed: https://crrev.com/6bf41bd974a754ee4e80f77099f4e0bf0bc793fa
> Cr-Commit-Position: refs/heads/master@{#361899}

TBR=dsinclair@chromium.org,primiano@chromium.org,picksi@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=555584
==========

ssid

Description was changed from ========== Reland of Fix regression in memory-infra bots due to change ...

5 years ago (2015-11-30 18:31:18 UTC) #3

Description was changed from

==========
Reland of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1479003003/ )

Reason for revert:
We have seen that the regression is fixed for renderer process, but not browser
process. The browser process regression is due to other CLs.
https://goo.gl/2I5xkG

Original issue's description:
> Revert of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1466293002/ )
> 
> Reason for revert:
> Along with this CL, the memory-infra build was broken
> (fixed in https://codereview.chromium.org/1474543003/). But, this
> cause much noise in the metrics and did not give a clear idea if the
> change fixed the original regression. So, I will revert this again
> and re-land it to check if that actually makes a difference.
> 
> Original issue's description:
> > Fix regression in memory-infra bots due to change in dump order
> >
> > crrev.com/1427963002 introduced a new memory-infra thread which runs
> > after all dump providers are finished. This measures the process total
> > memory usages. This was previously running at the start of a memory
> > dump. This means that the memory used while taking memory dump of other
> > providers will be included in the totals now and this metric has
> > regressed. This CL is a temporary change that reverts this behaviour so
> > that the perf bots are happy. Better fix for the problem will follow up
> > soon.
> >
> > BUG=555584
> >
> > Committed: https://crrev.com/ef89c2a502dadf4458000278ef4befbd79523bf8
> > Cr-Commit-Position: refs/heads/master@{#361126}
> 
> NOPRESUBMIT=true
> NOTREECHECKS=true
> BUG=555584
> 
> Committed: https://crrev.com/6bf41bd974a754ee4e80f77099f4e0bf0bc793fa
> Cr-Commit-Position: refs/heads/master@{#361899}

TBR=dsinclair@chromium.org,primiano@chromium.org,picksi@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=555584
==========

to

==========
Reland of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1479003003/ )

Reason for revert:
We have seen that the regression is fixed for renderer process, but
not browser process. The browser process regression is due to other CLs.
https://goo.gl/2I5xkG

Original issue's description:
> Revert of Fix regression in memory-infra bots due to change in dump order
(patchset #1 id:1 of https://codereview.chromium.org/1466293002/ )
> 
> Reason for revert:
> Along with this CL, the memory-infra build was broken
> (fixed in https://codereview.chromium.org/1474543003/). But, this
> cause much noise in the metrics and did not give a clear idea if the
> change fixed the original regression. So, I will revert this again
> and re-land it to check if that actually makes a difference.
> 
> Original issue's description:
> > Fix regression in memory-infra bots due to change in dump order
> >
> > crrev.com/1427963002 introduced a new memory-infra thread which runs
> > after all dump providers are finished. This measures the process total
> > memory usages. This was previously running at the start of a memory
> > dump. This means that the memory used while taking memory dump of other
> > providers will be included in the totals now and this metric has
> > regressed. This CL is a temporary change that reverts this behaviour so
> > that the perf bots are happy. Better fix for the problem will follow up
> > soon.
> >
> > BUG=555584
> >
> > Committed: https://crrev.com/ef89c2a502dadf4458000278ef4befbd79523bf8
> > Cr-Commit-Position: refs/heads/master@{#361126}
> 
> NOPRESUBMIT=true
> NOTREECHECKS=true
> BUG=555584
> 
> Committed: https://crrev.com/6bf41bd974a754ee4e80f77099f4e0bf0bc793fa
> Cr-Commit-Position: refs/heads/master@{#361899}

TBR=dsinclair@chromium.org,primiano@chromium.org,picksi@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=555584
==========

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1485813002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1485813002/1

5 years ago (2015-11-30 18:34:43 UTC) #5

Dry run: This issue passed the CQ dry run.

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages