Closed Bug 584182 Opened 11 years ago Closed 11 years ago

JM: Flush MICs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: adrake, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

Right now, below where PICs are flushed there's a commented out call to "FlushMICs" which doesn't exist.
Attached patch Implement FlushMICs. (obsolete) — Splinter Review
We only need to patch the shape guard for setgname and getgname, since the shapes could be changed during GC, and we may have an ABA problem. Exciting!
Attachment #465424 - Flags: review?(dvander)
Attached patch Implement PurgeMICs(). (obsolete) — Splinter Review
Same as before, minus debugging cruft. Whoops.
Attachment #465424 - Attachment is obsolete: true
Attachment #465429 - Flags: review?(dvander)
Attachment #465424 - Flags: review?(dvander)
Comment on attachment 465429 [details] [diff] [review]
Implement PurgeMICs().

Only a rare GC regenerates shapes (although we generate shapes like mad in Firefox, mainly due to XBL, currently). If nothing else in the MIC is a weak ref that might dangle unsafely, you shouldn't do this unless

cx->runtime->gcRegenShapes

is true. If other weak refs than shapes might dangle or replay badly, let's have a followup bug on invalidating them precisely (sorry if I missed one on file).

/be
Thanks, Brendan. This is the same as the previous patch, except that we respect cx->runtime->gcRegenShapes at the PurgeMICs callsite.
Attachment #465429 - Attachment is obsolete: true
Attachment #465454 - Flags: review?(dvander)
Attachment #465429 - Flags: review?(dvander)
Comment on attachment 465454 [details] [diff] [review]
Previous + only purge MICs if necessary


>+        /* MICs do not need to be purged unless shape guards are invalidated. */
>+        if (cx->runtime->gcRegenShapes)
>+            mjit::ic::PurgeMICs(cx, script);

This check already exists above, we don't purge PICs unless there's a shape-regenerating GC.

>+        ic::MICInfo &mic = script->mics[i];
>+        switch (mic.kind) {
>+          case ic::MICInfo::SET:
>+          case ic::MICInfo::GET:
>+            {

Align { with the "c" of "case", and 

>+                /* Patch shape guard. */

... this block to where the { was.
Attachment #465454 - Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/jaegermonkey/rev/c18a73b8fe64
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.