Bug 147908 – Always use a byte-sized lock implementation

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

[Filip Pizlo]

At time of writing, you could choose between Lock, which is a sizeof(void*) lock implementation with some nice theoretical properties, and ByteLock, which is a sizeof(uint8_t) lock implementation that empirically performs as well or better.

Clearly, we should make Lock use ByteLock's superior algorithm and get rid of ByteLock.

[Filip Pizlo]

Created attachment 258774 [details]
probably works

[Filip Pizlo]

Created attachment 258776 [details]
the patch

[Filip Pizlo]

Created attachment 258778 [details]
the patch

[Geoffrey Garen]

Comment on attachment 258778 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258778&action=review

r=me

> Source/WTF/ChangeLog:27
> +        - Move the algorithm in Lock into files called BasicLock.h|cpp. Make ParkingLot use
> +          BasicLock.

"Basic" doesn't say a lot to me about when to use or not use this lock. "Basic" feels good, but I probably shouldn't feel good about using this lock.

How about "ParkingLock". It helps to say "only the ParkingLot should use this".

Or, "WordLock" to describe a part of the tradeoff: bigger than a byte lock, so I probably don't want it.

[Filip Pizlo]

(In reply to comment #4)
> Comment on attachment 258778 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258778&action=review
> 
> r=me
> 
> > Source/WTF/ChangeLog:27
> > +        - Move the algorithm in Lock into files called BasicLock.h|cpp. Make ParkingLot use
> > +          BasicLock.
> 
> "Basic" doesn't say a lot to me about when to use or not use this lock.
> "Basic" feels good, but I probably shouldn't feel good about using this lock.
> 
> How about "ParkingLock". It helps to say "only the ParkingLot should use
> this".
> 
> Or, "WordLock" to describe a part of the tradeoff: bigger than a byte lock,
> so I probably don't want it.

I'll call it WordLock.

[Filip Pizlo]

Created attachment 258798 [details]
patch for landing

[WebKit Commit Bot]

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


ERROR: Source/WTF/benchmarks/LockSpeedTest.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 14 files


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

[Filip Pizlo]

(In reply to comment #7)
> Attachment 258798 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WTF/benchmarks/LockSpeedTest.cpp:29:  Alphabetical sorting
> problem.  [build/include_order] [4]
> Total errors found: 1 in 14 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Fixed locally.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/188323