1061646 - CheckForInterrupt is slower on windows

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Hannes Verschore [:h4writer]]

Looking on octane-regexp, CheckForInterrupt tend to take more time in windows than in linux. Especially in StrReplaceRegexpRemove.

I haven't really being able to figure out why yet.

Comment 1

[Luke Wagner [:luke]]

It might be worth looking at what assembly gets generated for the boolean test of cx->runtime->interrupt.  Its type is Atomic<bool, Relaxed> so it should just compile down to an unsynchronized load; but maybe something is messed up on windows.  (I mention this b/c we initially had a regression when someone changed 'interrupt' from a 'volatile bool' to an 'Atomic<bool>' which was fixed by changing to 'Atomic<bool, Relaxed>'.)

Comment 2

[Hannes Verschore [:h4writer]]

(In reply to Luke Wagner [:luke] from comment #1)
> It might be worth looking at what assembly gets generated for the boolean
> test of cx->runtime->interrupt.  Its type is Atomic<bool, Relaxed> so it
> should just compile down to an unsynchronized load; but maybe something is
> messed up on windows.  (I mention this b/c we initially had a regression
> when someone changed 'interrupt' from a 'volatile bool' to an 'Atomic<bool>'
> which was fixed by changing to 'Atomic<bool, Relaxed>'.)

Good suggestion, I'll look at that.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: Use MOZ_ALWAYS_INLINE instead of inline

Just force it to inline, though in most cases msvc already inlined it.

Comment 4

[Hannes Verschore [:h4writer]]

Like Luke already mentioned. This is because we lock on windows. I opened bug 1061764 for this.

Comment 5

[Hannes Verschore [:h4writer]]

Attachment: Workaround

This is a workaround which fixes this slowdown. (If mfbt cannot get adjusted to fix this or if there is no solution before the next train departs)

Comment 6

[Hannes Verschore [:h4writer]]

The depending bug landed, so this can get closed. I'll leave it open untill this small patch here lands.

Comment 7

[Jason Orendorff [:jorendorff]]

Comment on attachment 8482726 [details] [diff] [review]
Use MOZ_ALWAYS_INLINE instead of inline

Review of attachment 8482726 [details] [diff] [review]:
-----------------------------------------------------------------

Sure.

Comment 8

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5581850426c

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d5581850426c