-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix relative time in pthreads #9799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| // Copyright 2019 The Emscripten Authors. All rights reserved. | ||
| // Emscripten is available under two separate licenses, the MIT license and the | ||
| // University of Illinois/NCSA Open Source License. Both these licenses can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include <iostream> | ||
| #include <pthread.h> | ||
| #include <emscripten.h> | ||
|
|
||
| typedef std::chrono::milliseconds::rep TimePoint; | ||
|
|
||
| TimePoint now() { | ||
| return std::chrono::duration_cast<std::chrono::milliseconds>( | ||
| std::chrono::steady_clock::now().time_since_epoch()).count(); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think i'd find this test more readable and use fewer lines of code is it was just C and used |
||
|
|
||
| static TimePoint ping, pong; | ||
|
|
||
| static std::mutex mutex; | ||
| static std::condition_variable cond_var; | ||
|
|
||
| void *thread_main(void *arg) { | ||
| std::cout << "running thread ..." << std::endl; | ||
|
|
||
| std::unique_lock<std::mutex> lock(mutex); | ||
|
|
||
| cond_var.wait(lock); | ||
|
|
||
| // Measure time in the pthread. | ||
| pong = now(); | ||
|
|
||
| std::cout << "pong - ping: " << (now() - ping) << std::endl; | ||
|
|
||
| cond_var.notify_one(); | ||
|
|
||
| return NULL; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove all these extra newlines? |
||
| } | ||
|
|
||
| EMSCRIPTEN_KEEPALIVE | ||
| extern "C" int notify() { | ||
| { | ||
| std::unique_lock<std::mutex> lock(mutex); | ||
|
|
||
| std::cout << "notifying ..." << std::endl; | ||
|
|
||
| ping = now(); | ||
|
|
||
| cond_var.notify_one(); | ||
| } | ||
| { | ||
| std::unique_lock<std::mutex> lock(mutex); | ||
|
|
||
| cond_var.wait(lock); | ||
|
|
||
| TimePoint last = now(); | ||
|
|
||
| std::cout << "last - pong: " << (last - ping) << std::endl; | ||
|
|
||
| // Time measured on a worker should be relative to the main thread, | ||
| // so that things are basically monotonic. | ||
|
|
||
| REPORT_RESULT(int(pong >= ping) + 2 * int(last >= pong)); | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| int main() { | ||
| std::cout << "running main ..." << std::endl; | ||
|
|
||
| EM_ASM({ | ||
| setTimeout(function() { | ||
| Module._notify(); | ||
| }); | ||
| }); | ||
|
|
||
| pthread_attr_t attr; | ||
| pthread_attr_init(&attr); | ||
|
|
||
| pthread_t thread; | ||
| pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to set
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we cannot join the thread in this case, it seems best to explicitly detach it. Can try to remove, if you insist. |
||
| int error = pthread_create(&thread, &attr, thread_main, NULL); | ||
| if (error) | ||
| abort(); | ||
|
|
||
| return 0; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4567,6 +4567,13 @@ def run(emcc_args=[]): | |
| run(['-s', 'ASSERTIONS=1']) | ||
| run(['-s', 'PROXY_TO_PTHREAD=1']) | ||
|
|
||
| # Tests that time in a pthread is relative to the main thread, so measurements | ||
| # on different threads are still monotonic, as if checking a single central | ||
| # clock. | ||
| @requires_threads | ||
| def test_pthread_reltime(self): | ||
| self.btest(path_from_root('tests', 'pthread', 'test_pthread_reltime.cpp'), expected='3', args=['-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=1']) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this in node now too? We should probably have a plan for centralizing the thread tests that run in both browser and node. Perhaps
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to me, but since it seems like a larger undertaking I hope this does not block landing this fix. |
||
|
|
||
| # Tests that it is possible to load the main .js file of the application manually via a Blob URL, and still use pthreads. | ||
| @requires_threads | ||
| def test_load_js_from_blob_with_pthreads(self): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you wrap these comments at 80?