947499 - Create worker analogue to nsMainThreadPtrHolder

Status: RESOLVED INVALID

(Core :: DOM: Workers, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Bug 916893 uses a pattern similar to what nsMainThreadPtrHolder is designed for, except that the object is created on a worker thread, and held by the main thread and passed back to the worker to be released.

I'm going to modify nsMainThreadPtrHolder to be nsOwningThreadPtrHolder, which will assert accesses to the thread the object was created on, and release on that specific thread. It will hold a pointer to the creating thread instead of using the current NS_GetMainThread() utility functions.

Another option is to have a separate nsWorkerThreadPtrHolder, so that the extra pointer space is only used by worker specific code, and existing code using nsMainThreadPtrHolder can save that.

bholley, any thoughts on which option would be better?

Comment 1

[Bobby Holley (:bholley)]

I'll defer to bsmedberg, since this is his module.

Comment 2

[Benjamin Smedberg]

NS_IsMainThread is very fast because it's a single TLS lookup. Is PR_GetCurrentThread equivalently fast? This would at least cause nsMainThreadPtrHolder to be an extra word, which may not be a big deal because we don't use it that much.

Comment 3

[(no longer active)]

(In reply to comment #2)
> NS_IsMainThread is very fast because it's a single TLS lookup. Is
> PR_GetCurrentThread equivalently fast? This would at least cause
> nsMainThreadPtrHolder to be an extra word, which may not be a big deal because
> we don't use it that much.

PR_GetCurrentThread uses pthread_getspecific on pthreads and a __declspec(thread) variable on Windows, so it should be equally fast.

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Unfortunately the first option won't work. Workers have there own event loop which needs WorkerRunnable's, so nsProxyReleaseEvent can't be used. Similarly using just the current thread is not enough, since the WorkerThreadPtrHolder<> has to maintain a reference to the WorkerPrivate instead.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #4)
> which needs WorkerRunnable's

which need WorkerRunnables.

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Changed bug purpose to reflect changed and restricted use case. The (upcoming) patch for this is no longer useful for non-worker non-main threads.

Comment 7

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: WorkerThreadPtrHolder.

I ended up not needing this since the concept of features and worker lifetime restrictions makes it complicated to manage worker->main thread communication.
That is something not easily generalizable. Feel free to use this if its relevant for something else.