And replace WTF::ThreadCondition with WTF::Condition.
Created attachment 259190 [details] work in progress
Created attachment 259197 [details] the patch I plan to add some more things to this patch, like some better tests and benchmarks for the new Condition functionality. But right now this is passing all of our existing tests, which are pretty comprehensive.
Comment on attachment 259197 [details] the patch Attachment 259197 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/69504 Number of test failures exceeded the failure limit.
Created attachment 259207 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 259197 [details] the patch Attachment 259197 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/69508 Number of test failures exceeded the failure limit.
Created attachment 259210 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 259197 [details] the patch Clearing r? because EWS reports test crashes
(In reply to comment #7) > Comment on attachment 259197 [details] > the patch > > Clearing r? because EWS reports test crashes Fixed locally. It was obvious when I tested debug.
Created attachment 259232 [details] the patch Fixes crashes in the last patch. Adds a speed test.
Attachment 259232 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259236 [details] the patch Fix some builds.
Attachment 259236 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259239 [details] the patch Another build fix.
Attachment 259239 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259246 [details] the patch More build fixes for Linux, iOS, and Windows.
Attachment 259246 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/gstreamer/InbandTextTrackPrivateGStreamer.h:35: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 13 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259249 [details] the patch More build fixes.
Attachment 259249 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259250 [details] the patch Fix more things
Attachment 259250 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259252 [details] fix even more builds
Attachment 259252 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 259254 [details] the patch Bunch more Windows fixes.
Attachment 259254 [details] did not pass style-queue: ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:91: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:99: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:123: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:130: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:202: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:205: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:210: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:214: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:222: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:225: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:230: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WTF/benchmarks/ConditionSpeedTest.cpp:234: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 12 in 106 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 259254 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=259254&action=review r=me > Source/WTF/wtf/Condition.h:66 > + // May wait too long on some OSes: the C++ standard library implementation of > + // std::condition_variable::wait_until(), which this uses internally, may convert the absolute > + // time to an absolute time according to the system clock, which isn't monotonic. It will do this > + // so that it can use pthread_cond_timedwait(). Depending on the OS and if an unfortunate > + // combination of thread interleavings and system time changes happens, this may cause waitUntil() > + // to wait longer than it's supposed to. However, it's typical for OSes to turn the absolute > + // timeout in pthread_cond_timedwait() to a relative timeout before calling into the kernel. This > + // is what glibc does, for example. This seems more like documentation of an OS bug than of an intended quirk of the design here. Perhaps it doesn't belong here.
(In reply to comment #25) > Comment on attachment 259254 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259254&action=review > > r=me > > > Source/WTF/wtf/Condition.h:66 > > + // May wait too long on some OSes: the C++ standard library implementation of > > + // std::condition_variable::wait_until(), which this uses internally, may convert the absolute > > + // time to an absolute time according to the system clock, which isn't monotonic. It will do this > > + // so that it can use pthread_cond_timedwait(). Depending on the OS and if an unfortunate > > + // combination of thread interleavings and system time changes happens, this may cause waitUntil() > > + // to wait longer than it's supposed to. However, it's typical for OSes to turn the absolute > > + // timeout in pthread_cond_timedwait() to a relative timeout before calling into the kernel. This > > + // is what glibc does, for example. > > This seems more like documentation of an OS bug than of an intended quirk of > the design here. Perhaps it doesn't belong here. Fair enough, removed!
Landed in http://trac.webkit.org/changeset/188594