Bug 41792 – [WINCE] Implement TCSpinLock

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Patrick R. Gansterer]

InterlockedExchange is available on all windows platforms and can be in WinCE or as fallback:
http://msdn.microsoft.com/en-us/library/ms683590%28VS.85%29.aspx

[Patrick R. Gansterer]

Created attachment 60768 [details]
Patch

[WebKit Review Bot]

Attachment 60768 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/wtf/TCSpinLock.h:198:  "windows.h" already included at JavaScriptCore/wtf/TCSpinLock.h:55  [build/include] [4]
JavaScriptCore/wtf/TCSpinLock.h:200:  TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:205:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:210:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:214:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:215:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/TCSpinLock.h:225:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:225:  TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:226:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 9 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Roben (:aroben)]

Comment on attachment 60768 [details]
Patch

> +#elif OS(WINDOWS)

I don't understand how this code will be compiled for WinCE. The first branch of this #if/#elif chain is:

#if (CPU(X86) || CPU(X86_64) || CPU(PPC)) && (COMPILER(GCC) || COMPILER(MSVC))

Will WinCE not match that condition?

> +    LONG m_lockword;

I think m_lockword is supposed to be a volatile LONG.


> +static void TCMalloc_SlowLock(LPLONG lockword) {
> +    Sleep(0);        // Yield immediately since fast path failed
> +    while (true) {
> +        if (!InterlockedExchange(lockword, 1))
> +            return;
> +
> +        Sleep(2);
> +    }
> +}

This could be a little simpler:

Sleep(0);
while (InterlockedExchange(lockword, 1))
    Sleep(2);

[Patrick R. Gansterer]

Created attachment 62428 [details]
Patch

(In reply to comment #3)
> I don't understand how this code will be compiled for WinCE. The first branch of this #if/#elif chain is:
> 
> #if (CPU(X86) || CPU(X86_64) || CPU(PPC)) && (COMPILER(GCC) || COMPILER(MSVC))
> 
> Will WinCE not match that condition?
CPU(ARM)

> > +    LONG m_lockword;
> 
> I think m_lockword is supposed to be a volatile LONG.

No, otherwise you get "convert parameter 1 from 'volatile LONG *' to 'LPLONG'"

[WebKit Review Bot]

Attachment 62428 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/wtf/TCSpinLock.h:198:  "windows.h" already included at JavaScriptCore/wtf/TCSpinLock.h:55  [build/include] [4]
JavaScriptCore/wtf/TCSpinLock.h:200:  TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:205:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:210:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:214:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:215:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
JavaScriptCore/wtf/TCSpinLock.h:225:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
JavaScriptCore/wtf/TCSpinLock.h:225:  TCMalloc_SlowLock is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/TCSpinLock.h:226:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 9 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Roben (:aroben)]

(In reply to comment #4)
> Created an attachment (id=62428) [details]
> Patch
> 
> (In reply to comment #3)
> > I don't understand how this code will be compiled for WinCE. The first branch of this #if/#elif chain is:
> > 
> > #if (CPU(X86) || CPU(X86_64) || CPU(PPC)) && (COMPILER(GCC) || COMPILER(MSVC))
> > 
> > Will WinCE not match that condition?
> CPU(ARM)

Ah!

> > > +    LONG m_lockword;
> > 
> > I think m_lockword is supposed to be a volatile LONG.
> 
> No, otherwise you get "convert parameter 1 from 'volatile LONG *' to 'LPLONG'"

I guess InterlockedExchange has a different signature on WinCE.

[WebKit Commit Bot]

Comment on attachment 62428 [details]
Patch

Clearing flags on attachment: 62428

Committed r63981: <http://trac.webkit.org/changeset/63981>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.