522361 - Minor String-creation optimizations

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

Attachment: Patch

Based on profiling some code that does many, many, many dependent-string concatenations, a few minor optimizations that produce meaningful speedups (>10% for this pathological case). Please review carefully, just in case I misunderstand the subtlety of when WB/WBRC is needed...

(1) In the dynamic-String ctor, we use an explicit WB call, but this should only be necessary if there's the possibility of an allocation between the time the container (String) is allocated and time we set the field. In Debugger builds, the superclass ctor (AvmPlusScriptableObject) could do so, but otherwise, we shouldn't need to call WB, we can just assign the field directly.

(2) In the dependent-String ctor, we use an explicit WBRC call, but as above, there's no possibility of allocation in nonDebugger builds. We do need to call IncrementRef (but the value is always non-null).

(3) String::createDependent is basically a wrapper around the ctor, and should be declared REALLY_INLINE.

Comment 1

[Lars T Hansen]

Comment on attachment 406319 [details] [diff] [review]
Patch

Based on the conventions we've used before, this should be OK: fields stored during object initialization don't need a barrier.  I don't really understand why you left the barrier for the DEBUGGER case though, since (so far as I can tell) there are no debugger hooks inside the write barrier.

Comment 2

[Steven Johnson]

(In reply to comment #1)
> (From update of attachment 406319 [details] [diff] [review])
> why you left the barrier for the DEBUGGER case though, since (so far as I can
> tell) there are no debugger hooks inside the write barrier.

There are debugger hooks inside the superclass ctor: if the sampler is enabled, we record allocation information, which can allocate memory. (I think: let me doublecheck.)

Comment 3

[Lars T Hansen]

(In reply to comment #2)

> There are debugger hooks inside the superclass ctor: if the sampler is enabled,
> we record allocation information, which can allocate memory. (I think: let me
> doublecheck.)

Icky brittleness.  Should document the reason for the strange code pattern then.

Comment 4

[Steven Johnson]

(In reply to comment #3)
> Icky brittleness.  Should document the reason for the strange code pattern
> then.

No argument here. I'll add appropriate comments.

Comment 5

[Steven Johnson]

pushed as changeset:   2811:8280ba31e664