Bobby Holley (:bholley) Assignee
	Description • 7 years ago

Brian Lewis discovered this in [1]. Thanks Brian!

Working up a patch now.

[1] https://github.com/servo/servo/issues/17280

	Bobby Holley (:bholley) Assignee
	Comment 1 • 7 years ago

Oh, and on top of that, it looks like we never actually call maybe_gc(). Adding a patch for that as well...

	Bobby Holley (:bholley) Assignee
	Comment 2 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=83d058d26ec45d43916e8cd91c8c55fc78ce40f4

	Bobby Holley (:bholley) Assignee
	Comment 3 • 7 years ago

And here's a try run with a bonus fix to assert against permanently leaking rule nodes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd0ca5e6e654f638f3bceafb56ee6e9d566e7006

	Bobby Holley (:bholley) Assignee
	Comment 4 • 7 years ago

MozReview-Commit-ID: 1uipYlIF8fv

	Bobby Holley (:bholley) Assignee
	Comment 5 • 7 years ago

MozReview-Commit-ID: 9jVkDM4P8mC

	Bobby Holley (:bholley) Assignee
	Comment 6 • 7 years ago

MozReview-Commit-ID: W2lkQohudA

	Bobby Holley (:bholley) Assignee
	Comment 7 • 7 years ago

MozReview-Commit-ID: CK5iEWWHFSr

	Emilio Cobos Álvarez (:emilio)
	Comment 8 • 7 years ago

Comment on attachment 8878636 [details] [diff] [review]
Part 1 - Actually increment the counter when adding rule nodes to the free list. v1

Review of attachment 8878636 [details] [diff] [review]:
-----------------------------------------------------------------

Pfft... good catch :)

	Emilio Cobos Álvarez (:emilio)
	Comment 9 • 7 years ago

Comment on attachment 8878638 [details] [diff] [review]
Part 3 - Trigger a rule tree gc at the end of DoProcessPendingRestyles. v1

Review of attachment 8878638 [details] [diff] [review]:
-----------------------------------------------------------------

I'm pretty sure we called this when it was added, I wonder when was it removed...

	Emilio Cobos Álvarez (:emilio)
	Comment 10 • 7 years ago

Comment on attachment 8878639 [details] [diff] [review]
Part 4 - Assert against permanently-leaked rule nodes. v1

Review of attachment 8878639 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

	Emilio Cobos Álvarez (:emilio)
	Comment 11 • 7 years ago

Ick, of course I was the one that removed it... https://github.com/servo/servo/commit/e67ea42c3fa03264b8c897430f7c17a7c153e5bd

	Bobby Holley (:bholley) Assignee
	Comment 12 • 7 years ago

https://github.com/servo/servo/pull/17368

	Bobby Holley (:bholley) Assignee
	Comment 13 • 7 years ago

https://hg.mozilla.org/integration/autoland/rev/1f216a97ac0633e2ffd5f9af2893e4610f7206f7

	Pulsebot
	Comment 14 • 7 years ago

Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a21be24aa822
Trigger a rule tree gc at the end of DoProcessPendingRestyles. r=emilio

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 15 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/a21be24aa822