Minor String-creation optimizations

VERIFIED FIXED

Status

Tamarin
Virtual Machine
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Steven Johnson, Assigned: Steven Johnson)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Created attachment 406319 [details] [diff] [review]
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.
Attachment #406319 - Flags: review?(lhansen)

Comment 1

8 years ago
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.
Attachment #406319 - Flags: review?(lhansen) → review+
(Assignee)

Comment 2

8 years ago
(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

8 years ago
(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.
(Assignee)

Comment 4

8 years ago
(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.
(Assignee)

Comment 5

8 years ago
pushed as changeset:   2811:8280ba31e664
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.