585431 - implement threadsafety pieces of harfbuzz for win32

Status: RESOLVED INCOMPLETE

(Core :: Graphics, defect)

Description

[timeless]

this implementation is based on the work for bug 585425. But I haven't asked a compiler what it thinks.

Comment 1

[timeless]

Attachment: untested

Comment 2

[Bas Schouten (:bas.schouten)]

I suggest using a CRITICAL_SECTION instead of a mutex. CRITICAL_SECTIONS do not trigger kernel mode switches when they are free.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

yep, second CRITICAL_SECTION suggestion.  Also, we currently do rendering on only one thread, and already have cairo's thread safety disabled.  We should see what kind of perf hit (if any) this stuff gives, if we don't actually need it.

Comment 4

[timeless]

Attachment: untested

ok, this is probably what critical_sections would look like. again i don't have a compiler handy.

Comment 5

[Bas Schouten (:bas.schouten)]

(In reply to comment #4)
> Created attachment 464168 [details] [diff] [review]
> untested
> 
> ok, this is probably what critical_sections would look like. again i don't have
> a compiler handy.

I haven't tested it, and it's certainly what it would look like. However do we really want a spin count that high? Seems to me like for a general purpose implementation like this we just want to stick at 0 or something low like that. This could potentially spin for quite a while.

Comment 6

[timeless]

the 0x8 bit is magical, the other bit isn't that big:

https://forums.codegear.com/thread.jspa?threadID=17554

Comment 7

[Jonathan Kew [:jfkthame]]

I don't think we should be patching this for now, as we don't actually need it. See bug 585425 comment #3.

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 464168 [details] [diff] [review]
untested

Cancelling review and jfkthame says we don't need this for now, please feel free to re-request if this situation changes of course.

Comment 9

[Behdad Esfahbod]

Upstream HarfBuzz supports win32 threadsafety already.  This can be closed.

Comment 10

[Joe Drew (not getting mail)]

sounds good to me.