Skip to content
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

feature(neon): API for thread-local data #902

Merged
merged 33 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
94111d4
Add `neon::instance::Global` API for storing instance-global data.
dherman May 24, 2022
f872b76
Address review comments:
dherman May 25, 2022
6f12663
Use `'cx` as the inner lifetime of contexts.
dherman May 25, 2022
18e60f1
Get rid of `[]` overloading for `GlobalTable` in favor of an inherent…
dherman May 25, 2022
b8b089b
Immutable version of API:
dherman May 25, 2022
cf0d32a
Explicitly name the types in the transmute
dherman May 25, 2022
f5cd0ba
Explicitly name the types in the transmute.
dherman May 25, 2022
aee77f4
Add `get_or_try_init` and `get_or_init_default`
dherman May 27, 2022
9075b76
Protect re-entrant cases with "dirty" state checking
dherman Jun 1, 2022
b28f439
Use `GlobalCellValue` shorthand in type definition of `GlobalCell`.
dherman Jun 1, 2022
bbc4f22
Prettier fixups
dherman Jun 1, 2022
ff75d2d
Add a test for storing rooted objects in instance globals
dherman Jun 1, 2022
d9b8251
Minor style cleanup for `TryInitTransaction::is_trying()`
dherman Jun 1, 2022
a66e511
Global::new() can use the derived Default::default()
dherman Jun 2, 2022
507332d
Undo previous commit, which failed to type-check.
dherman Jun 2, 2022
0696901
Test that inner closure isn't even executed on re-entrant initializat…
dherman Jun 2, 2022
574877b
Make `get_or_try_init` generic for any `Result<T, E>`
dherman Jun 2, 2022
b397e2d
Change "safety" to "unwrap safety" for the `.unwrap()` comment.
dherman Jun 2, 2022
bc7d09e
Use `get_or_try_init` for the global object test, to only root the ob…
dherman Jun 2, 2022
3a0c041
Rename `Global` to `Local` and add top-level API docs for the `neon::…
dherman Jun 3, 2022
3338877
Improvements to `neon::instance` top-level API docs
dherman Jun 8, 2022
42eec50
Rename `neon::instance` to `neon::thread` and `Local` to `LocalKey`
dherman Jun 8, 2022
85d99f5
Some more documentation text about the relationships between instance…
dherman Jun 8, 2022
c906fbc
Addresses some of @kjvalencik's review suggestions:
dherman Jun 9, 2022
0f57620
Clarify doc text
dherman Jun 9, 2022
14a2fe4
Idiomatic Rust variable name in doc example
dherman Jun 9, 2022
019c2d3
Link to `neon::main` docs in doc comment
dherman Jun 9, 2022
58f80b3
Clarifying doc text about cross-thread sharing
dherman Jun 9, 2022
f3cc0ab
s/fail/panic/ in doc text
dherman Jun 9, 2022
2005e61
More docs improvements:
dherman Jun 9, 2022
d50ca63
Link to `crate::main` in the docs instead of `neon::main`
dherman Jun 9, 2022
ece0c02
More copy editing in the docs
dherman Jun 9, 2022
310cb5b
A few last copy-editing nits
dherman Jun 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions crates/neon/src/instance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,33 @@ impl<T> Global<T> {
}

impl<T: Any + Send + 'static> Global<T> {
dherman marked this conversation as resolved.
Show resolved Hide resolved
pub fn get<'cx, 'a, C>(&self, cx: &'a mut C) -> Option<&'a T>
pub fn get<'cx, 'a, C>(&self, cx: &'a mut C) -> Option<&'cx T>
where
C: Context<'cx>,
{
InstanceData::globals(cx)
let r: Option<&T> = InstanceData::globals(cx)
.get(self.id())
.as_ref()
.map(|boxed| boxed.downcast_ref().unwrap())
.map(|boxed| boxed.downcast_ref().unwrap());

unsafe {
std::mem::transmute(r)
}
}

pub fn get_mut<'cx, 'a, C>(&self, cx: &'a mut C) -> Option<&'a mut T>
pub fn get_or_init<'cx, 'a, C, F>(&self, cx: &'a mut C, f: F) -> &'cx T
dherman marked this conversation as resolved.
Show resolved Hide resolved
where
C: Context<'cx>,
F: FnOnce() -> T,
{
InstanceData::globals(cx)
let r: &T = InstanceData::globals(cx)
.get(self.id())
.as_mut()
.map(|boxed| boxed.downcast_mut().unwrap())
}
.get_or_insert_with(|| Box::new(f()))
.downcast_ref()
.unwrap();

pub fn set<'cx, 'a, C>(&self, cx: &'a mut C, v: T)
where
C: Context<'cx>,
{
*InstanceData::globals(cx).get(self.id()) = Some(Box::new(v));
unsafe {
std::mem::transmute(r)
dherman marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
15 changes: 9 additions & 6 deletions test/napi/lib/workers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const addon = require("..");

// Receive a message, try that method and return the error message
if (!isMainThread) {
addon.set_thread_id(threadId);
addon.get_or_init_thread_id(threadId);
parentPort.once("message", (message) => {
try {
switch (message) {
Expand All @@ -20,7 +20,7 @@ if (!isMainThread) {
break;
case "get_thread_id":
{
let id = addon.get_thread_id();
let id = addon.get_or_init_thread_id(NaN);
parentPort.postMessage(id);
}
break;
Expand All @@ -40,7 +40,7 @@ if (!isMainThread) {
// From here on, we're in the main thread.

// Set the `THREAD_ID` Global value in the main thread cell.
addon.set_thread_id(threadId);
addon.get_or_init_thread_id(threadId);

describe("Worker / Root Tagging Tests", () => {
describe("Single Threaded", () => {
Expand Down Expand Up @@ -121,19 +121,22 @@ describe("Worker / Root Tagging Tests", () => {

describe("Globals", () => {
it("should be able to read an instance global from the main thread", () => {
let lookedUpId = addon.get_thread_id();
let lookedUpId = addon.get_or_init_thread_id(NaN);
assert(!Number.isNaN(lookedUpId));
assert.strictEqual(lookedUpId, threadId);
});

it("should allocate separate globals for each addon instance", (cb) => {
let mainThreadId = addon.get_thread_id();
let mainThreadId = addon.get_or_init_thread_id(NaN);
assert(!Number.isNaN(mainThreadId));

const worker = new Worker(__filename);

worker.once("message", (message) => {
assert.strictEqual(typeof message, 'number');
assert.notStrictEqual(message, mainThreadId);
let mainThreadIdAgain = addon.get_thread_id();
let mainThreadIdAgain = addon.get_or_init_thread_id(NaN);
assert(!Number.isNaN(mainThreadIdAgain));
assert.strictEqual(mainThreadIdAgain, mainThreadId);
cb();
});
Expand Down
14 changes: 3 additions & 11 deletions test/napi/src/js/workers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,8 @@ pub fn get_or_init_clone(mut cx: FunctionContext) -> JsResult<JsObject> {

static THREAD_ID: Global<u32> = Global::new();

pub fn set_thread_id(mut cx: FunctionContext) -> JsResult<JsUndefined> {
pub fn get_or_init_thread_id(mut cx: FunctionContext) -> JsResult<JsNumber> {
let id = cx.argument::<JsNumber>(0)?.value(&mut cx) as u32;
THREAD_ID.set(&mut cx, id);
Ok(cx.undefined())
}

pub fn get_thread_id(mut cx: FunctionContext) -> JsResult<JsValue> {
let id = THREAD_ID.get(&mut cx).cloned();
Ok(match id {
Some(id) => cx.number(id).upcast(),
None => cx.undefined().upcast(),
})
let id: &u32 = THREAD_ID.get_or_init(&mut cx, || id);
Ok(cx.number(*id))
}
3 changes: 1 addition & 2 deletions test/napi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
cx.export_function("get_and_replace", js::workers::get_and_replace)?;
cx.export_function("get_or_init", js::workers::get_or_init)?;
cx.export_function("get_or_init_clone", js::workers::get_or_init_clone)?;
cx.export_function("get_thread_id", js::workers::get_thread_id)?;
cx.export_function("set_thread_id", js::workers::set_thread_id)?;
cx.export_function("get_or_init_thread_id", js::workers::get_or_init_thread_id)?;

Ok(())
}