Closed Bug 1214173 Opened 6 years ago Closed 6 years ago

Remove MSetPropertyInstruction::needsBarrier_

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
It's never read (and always `true`).
Attachment #8673047 - Flags: review?(hv1989)
Comment on attachment 8673047 [details] [diff] [review]
Patch

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

There seems to be an inconsistency between MSetPropertyInstruction and MSetPropertyCache.

MSetPropertyInstruction has a needsBarrier_ which is always true. (potentially for child classes to override).
MSetPropertyCache inherit MSetPropertyInstruction, but does not re-use the needsBarrier_. It creates a new variable needsTypeBarrier_

now every other class uses needsBarrier_ instead of needsTypeBarrier_ as name. Therefore can we also rename the MSetPropertyCache needsTypeBarrier_ variable to needsBarrier_
Attachment #8673047 - Flags: review?(hv1989) → review+
(In reply to Hannes Verschore [:h4writer] from comment #1)
> now every other class uses needsBarrier_ instead of needsTypeBarrier_ as
> name. Therefore can we also rename the MSetPropertyCache needsTypeBarrier_
> variable to needsBarrier_

It's confusing but I think needsBarrier_ refers to a pre-barrier for incremental GC and needsTypeBarrier_ refers to a TypeSet barrier. We should probably rename needsBarrier_ to be more explicit some day...
https://hg.mozilla.org/mozilla-central/rev/6c4f31135970
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.