628369 - ScriptObject::gc() is inefficient

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

It is implemented as

    return vtable->traits->core->GetGC();

But would be more efficient as

    return MMgc::GC::GetGC(this);

Comment 1

[Tommy Reilly]

GC::GetGC(GCObject*) has always been looked upon with derision b/c its a theoretical impediment to different allocation schemes (such as a bump pointer nursery).    As always perf #'s speak volumes.

Comment 2

[Edwin Smith]

Many of our key api objects have a gc() method for getting the gc whatever way is fastest, even if the implementation changes.  We're only talking about a handful of accessor methods that would have to change.

If bug 619858 lands they all can go away.

Comment 3

[Lars T Hansen]

(In reply to comment #1)

> GC::GetGC(GCObject*) has always been looked upon with derision b/c its a
> theoretical impediment to different allocation schemes (such as a bump pointer
> nursery).

I don't see how it's true that it's an impediment, though it may be true for the current implementation of GC::GetGC in 4K pages.  One could imagine larger allocation units if we want to keep the implementation, or the implementation could go via the page table, which would obviously be quite a bit slower.

Comment 4

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #3)
> or the implementation
> could go via the page table, which would obviously be quite a bit slower.

Note that would require implementing something like the scheme in Bug 610982.  Currently you need a GC* in order to get your hands on the page table.

Comment 5

[Steven Johnson]

My main point is that with the current implementation, it's almost inconceivable that the GetGC(this) approach wouldn't be fewer loads. (But yeah, no substiture for measuring.) If/when GetGC(this) becomes invalid, we change the impl of gc().

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.