search

neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

Shared ThreadsafeFunction in EventQueue RFC #42

Closed indutny closed 3 years ago

indutny commented 3 years ago

See: https://github.com/neon-bindings/neon/pull/739

indutny commented 3 years ago

@indutny Thanks for providing this. I think this is a really interesting idea.

One complexity concern I have is how dropping a referenced EventQueue would work. With a single shared queue, the queue needs to track how many times it has been referenced. When the count drops to 0, the shared queue needs to unreference itself.

However, unreferencing can only be performed on the event loop. The only solution I can think of is sending an event to unreference itself. This could end up being costly if referenced queues are frequently created and dropped.

This indeed the case, but there's little to no performance penalty for doing it this way with the shared ThreadsafeFunction because it is stupid fast in Node now :joy: I've added an optimization to my PR to avoid this extra work for the EventQueue::new() because it owns the ThreadsafeFunction and will drop it automatically.

Thank you so much for a thorough review. I believe I've addressed your feedback. PTAL.

kjvalencik commented 3 years ago

Closing because the contents have been merged into https://github.com/neon-bindings/rfcs/pull/32