Update the post-barrier verifier

RESOLVED FIXED in mozilla24

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

Trunk
mozilla24
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch v0Splinter Review
Now that the nursery is working, we can use it to implement the post-barrier verifier. This lets us strip out gcVerifierNursery and attendant ugliness for a savings of:

14 files changed, 112 insertions(+), 322 deletions(-)

It also tremendously speeds up the verification: with this patch it is about 1/2 the speed of normal execution. This should be usable for testing the browser's barriers, and possibly even fast enough to run on tbpl.
Attachment #764480 - Flags: review?(jcoppeard)
Attachment #764480 - Flags: feedback?(wmccloskey)
Comment on attachment 764480 [details] [diff] [review]
v0

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

Looks good!

A couple of minor comments:

I needed to add |#include "jsgcinlines.h"| in Verifier.cpp to get the patch to compile, to pull in the definition of IsInsideNursery().

AutoVerifyBarriers looks like this disables verification for its lifetime - could this be named something that makes this more obvious (e.g. AutoStopVerifyingBarriers)?
Attachment #764480 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #1)
> Comment on attachment 764480 [details] [diff] [review]
> v0
> 
> Review of attachment 764480 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good!
> 
> A couple of minor comments:
> 
> I needed to add |#include "jsgcinlines.h"| in Verifier.cpp to get the patch
> to compile, to pull in the definition of IsInsideNursery().

And several others. IWYU keeps removing the includes required for ggc, since it doesn't know about it.

> AutoVerifyBarriers looks like this disables verification for its lifetime -
> could this be named something that makes this more obvious (e.g.
> AutoStopVerifyingBarriers)?

Excellent idea! Updated.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a72d816724da
https://hg.mozilla.org/mozilla-central/rev/a72d816724da
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Attachment #764480 - Flags: feedback?(wmccloskey) → feedback+
You need to log in before you can comment on or make changes to this bug.