-
Notifications
You must be signed in to change notification settings - Fork 462
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
Implement AsyncProgressQueueWorker #585
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
630c005
Implement AsyncProgressWorkerBase
legendecas 01d157c
Implement AsyncProgressQueueWorker
legendecas c558b42
asyncworker: wait for tsfn completing
legendecas 95890af
test: cancel a queue worker
legendecas 0ced771
doc: async progress queue worker
legendecas baa03a5
fix gcc complains about unused parameters
legendecas db62dfb
fix flaky async queue worker cancelling case
legendecas a8e76c4
fixup chore issues
legendecas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -272,12 +272,12 @@ called and are executed as part of the event loop. | |||||||
The code below shows a basic example of the `Napi::AsyncProgressWorker` implementation: | ||||||||
|
||||||||
```cpp | ||||||||
#include<napi.h> | ||||||||
#include <napi.h> | ||||||||
|
||||||||
#include <chrono> | ||||||||
#include <thread> | ||||||||
|
||||||||
use namespace Napi; | ||||||||
using namespace Napi; | ||||||||
|
||||||||
class EchoWorker : public AsyncProgressWorker<uint32_t> { | ||||||||
public: | ||||||||
|
@@ -323,7 +323,7 @@ The following code shows an example of how to create and use an `Napi::AsyncProg | |||||||
// Include EchoWorker class | ||||||||
// .. | ||||||||
|
||||||||
use namespace Napi; | ||||||||
using namespace Napi; | ||||||||
|
||||||||
Value Echo(const CallbackInfo& info) { | ||||||||
// We need to validate the arguments here | ||||||||
|
@@ -341,4 +341,116 @@ asynchronous task ends and other data needed for the computation. Once created, | |||||||
the only other action needed is to call the `Napi::AsyncProgressWorker::Queue` | ||||||||
method that will queue the created worker for execution. | ||||||||
|
||||||||
# AsyncProgressQueueWorker | ||||||||
|
||||||||
`Napi::AsyncProgressQueueWorker` acts exactly like `Napi::AsyncProgressWorker` | ||||||||
except that each progress committed by `Napi::AsyncProgressQueueWorker::ExecutionProgress::Send` | ||||||||
during `Napi::AsyncProgressQueueWorker::Execute` is guaranteed to be | ||||||||
processed by `Napi::AsyncProgressQueueWorker::OnProgress` on the JavaScript | ||||||||
thread in the order it was committed. | ||||||||
|
||||||||
For the most basic use, only the `Napi::AsyncProgressQueueWorker::Execute` and | ||||||||
`Napi::AsyncProgressQueueWorker::OnProgress` method must be implemented in a subclass. | ||||||||
|
||||||||
# AsyncProgressQueueWorker::ExecutionProcess | ||||||||
|
||||||||
A bridge class created before the worker thread execution of `Napi::AsyncProgressQueueWorker::Execute`. | ||||||||
|
||||||||
## Methods | ||||||||
|
||||||||
### Send | ||||||||
|
||||||||
`Napi::AsyncProgressQueueWorker::ExecutionProcess::Send` takes two arguments, a pointer | ||||||||
to a generic type of data, and a `size_t` to indicate how many items the pointer is | ||||||||
pointing to. | ||||||||
|
||||||||
The data pointed to will be copied to internal slots of `Napi::AsyncProgressQueueWorker` so | ||||||||
after the call to `Napi::AsyncProgressQueueWorker::ExecutionProcess::Send` the data can | ||||||||
be safely released. | ||||||||
|
||||||||
`Napi::AsyncProgressQueueWorker::ExecutionProcess::Send` guarantees invocation | ||||||||
of `Napi::AsyncProgressQueueWorker::OnProgress`, which means multiple `Send` | ||||||||
call will result in the in-order invocation of `Napi::AsyncProgressQueueWorker::OnProgress` | ||||||||
with each data item. | ||||||||
|
||||||||
```cpp | ||||||||
void Napi::AsyncProgressQueueWorker::ExecutionProcess::Send(const T* data, size_t count) const; | ||||||||
``` | ||||||||
|
||||||||
## Example | ||||||||
|
||||||||
The code below shows a basic example of the `Napi::AsyncProgressQueueWorker` implementation: | ||||||||
|
||||||||
```cpp | ||||||||
#include <napi.h> | ||||||||
|
||||||||
#include <chrono> | ||||||||
#include <thread> | ||||||||
|
||||||||
using namespace Napi; | ||||||||
|
||||||||
class EchoWorker : public AsyncProgressQueueWorker<uint32_t> { | ||||||||
public: | ||||||||
EchoWorker(Function& callback, std::string& echo) | ||||||||
: AsyncProgressQueueWorker(callback), echo(echo) {} | ||||||||
|
||||||||
~EchoWorker() {} | ||||||||
// This code will be executed on the worker thread | ||||||||
void Execute(const ExecutionProgress& progress) { | ||||||||
// Need to simulate cpu heavy task | ||||||||
for (uint32_t i = 0; i < 100; ++i) { | ||||||||
progress.Send(&i, 1) | ||||||||
std::this_thread::sleep_for(std::chrono::seconds(1)); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
void OnOK() { | ||||||||
HandleScope scope(Env()); | ||||||||
Callback().Call({Env().Null(), String::New(Env(), echo)}); | ||||||||
} | ||||||||
|
||||||||
void OnProgress(const uint32_t* data, size_t /* count */) { | ||||||||
HandleScope scope(Env()); | ||||||||
Callback().Call({Env().Null(), Env().Null(), Number::New(Env(), *data)}); | ||||||||
} | ||||||||
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.
Suggested change
if we go with the |
||||||||
|
||||||||
private: | ||||||||
std::string echo; | ||||||||
}; | ||||||||
``` | ||||||||
|
||||||||
The `EchoWorker`'s constructor calls the base class' constructor to pass in the | ||||||||
callback that the `Napi::AsyncProgressQueueWorker` base class will store | ||||||||
persistently. When the work on the `Napi::AsyncProgressQueueWorker::Execute` | ||||||||
method is done the `Napi::AsyncProgressQueueWorker::OnOk` method is called and | ||||||||
the results are returned back to JavaScript when the stored callback is invoked | ||||||||
with its associated environment. | ||||||||
|
||||||||
The following code shows an example of how to create and use an | ||||||||
`Napi::AsyncProgressQueueWorker`. | ||||||||
|
||||||||
```cpp | ||||||||
#include <napi.h> | ||||||||
|
||||||||
// Include EchoWorker class | ||||||||
// .. | ||||||||
|
||||||||
using namespace Napi; | ||||||||
|
||||||||
Value Echo(const CallbackInfo& info) { | ||||||||
// We need to validate the arguments here. | ||||||||
Function cb = info[1].As<Function>(); | ||||||||
std::string in = info[0].As<String>(); | ||||||||
EchoWorker* wk = new EchoWorker(cb, in); | ||||||||
wk->Queue(); | ||||||||
return info.Env().Undefined(); | ||||||||
} | ||||||||
``` | ||||||||
|
||||||||
The implementation of a `Napi::AsyncProgressQueueWorker` can be used by creating a | ||||||||
new instance and passing to its constructor the callback to execute when the | ||||||||
asynchronous task ends and other data needed for the computation. Once created, | ||||||||
the only other action needed is to call the `Napi::AsyncProgressQueueWorker::Queue` | ||||||||
method that will queue the created worker for execution. | ||||||||
|
||||||||
[`Napi::AsyncWorker`]: ./async_worker.md |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does this really do what we think it does? After all, we send the same pointer multiple times, so at the point of retrieval it will have the value it currently has on the thread. Also, if
i
goes out of scope, the main thread might crash. How aboutand then we delete on the main thread?
Nit: missing semcolon.
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.
Internally, the passed in data items are copied to internal slots and these allocations are managed by AsyncProgressQueueWorker.
Exposing passing raw pointers may have better control over the
Send
operation, but it also made the lifetime management of these data mandatory. Users have to manually delete the data pointers after the progress been handled.I'm leaning on the current behavior (copy the data) and this is what NAN provides. WDYT?