Bug 166989 - We could probably remove the write barrier at the end Structure::flattenDictionaryStructure
Summary: We could probably remove the write barrier at the end Structure::flattenDicti...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on: 166960
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-12 14:54 PST by Saam Barati
Modified: 2018-06-19 17:43 PDT (History)
11 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-01-12 14:54:56 PST
After https://bugs.webkit.org/show_bug.cgi?id=166960, we'll have a way that rescans after races are detected.
Comment 1 Saam Barati 2018-06-19 17:39:23 PDT
I actually think this is necessary, imagine you're in the middle of flatten, and you have a butterfly that contains 3 valid properties, and a bunch of empty space, like this:
{a, ..., b, ..., c, ...}

and we want to end up with our property storage looking like this while in the middle of this loop:
```
        // Copies in our values to their compacted locations.
        for (unsigned i = 0; i < propertyCount; i++)
            object->putDirect(vm, offsetForPropertyNumber(i, m_inlineCapacity), values[i]);
```
{..., a, ..., a, c}

And imagine if we scanned the property storage at that point, we'd end not scanning for "b". Therefore, I think this barrier is indeed needed.
Comment 2 Saam Barati 2018-06-19 17:43:08 PDT
(In reply to Saam Barati from comment #1)
> I actually think this is necessary, imagine you're in the middle of flatten,
> and you have a butterfly that contains 3 valid properties, and a bunch of
> empty space, like this:
> {a, ..., b, ..., c, ...}
> 
> and we want to end up with our property storage looking like this while in
> the middle of this loop:
> ```
>         // Copies in our values to their compacted locations.
>         for (unsigned i = 0; i < propertyCount; i++)
>             object->putDirect(vm, offsetForPropertyNumber(i,
> m_inlineCapacity), values[i]);
> ```
> {..., a, ..., a, c}
> 
> And imagine if we scanned the property storage at that point, we'd end not
> scanning for "b". Therefore, I think this barrier is indeed needed.

The reason we may not detect this as a race is like so:

Imagine the collector did this:
ReadStructureEarly
ReadLastOffsetEarly
ReadButterfly
ReadStructureLate
ReadLastOffsetLate
then, the mutator does
flattenDictionaryStructure
mutator gets into the middle of the above loop
collector scans property storage

We won't detect a race here, but we need to rescan.