[Webkit-unassigned] [Bug 60950] Port the SmallStrings cache to Weak<T>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60950





--- Comment #3 from Oliver Hunt <oliver at apple.com>  2011-05-17 18:36:50 PST ---
(In reply to comment #2)
> > - As a test to see if I could make it actually work I ported everything to Weak<T> and made the JIT try to get the JSString from inside the Weak<T> now exposed in the API.
> 
> That sounds fine. It just adds one pointer load to the cost of charCodeAt.

It's also used for string indexing, see the stringGetByValStubGenerator() function.

We would have to maintain a live handle for every slot for all time, each handle is in the order of 64bytes so spans a cache line so the indirection is more likely to add an additional cache miss, and the you blow locality.

When I originally wrote this code i found that it was fairly sensitive to exact instruction ordering.

I'm not saying that there is guaranteed to be cost, but "just one pointer load" does a lot.  It may have even more impact on an in-order architecture.

> 
> Did you measure the performance of your patch?

My recommended tests would be manually unrolled calls to charAt(), charCodeAt(), and simple string indexing.

something akin to
function test(str) {
    for (var i = 0; i < 100000; i++) {
        str[0];
        str[1];
        str[2];
        str[3];
    }
}
test("foobar");

> 
> > This seems to work (in some cases), but I'm not sure if it makes any sense. In particular, I understand that the data behind a Weak<T> can change at any moment as a side effect of GC, so perhaps it just does not make any sense to generate code that does this.
> 
> If the JIT loads the JSString from the Weak<T>, it won't matter if the JSString in the Weak<T> has changed -- the JIT will correctly see the new value.
> 
> Is something going wrong in your code? It would be easier to diagnose if you posted your patch.
> 
> > I'm not sure who put that there, 
> 
> I did.
> 
> > If it's really necessary to have weak behaviour, we have sufficient control of this data to implement weak semantics directly in the buffer.
> 
> We have that now, but the implementation is a bit awkward. If the cost of an extra pointer load in these cases is not measurable, I think it would be an improvement to switch over to the more general Weak<T> implementation.

I suspect this would also be a memory hit as each handle is 6 pointers + a JSValue so 8k on 32bit and 14k on 64bit platforms (assuming there's no alignment related padding happening in HandleHeap::Node).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.