Currently, each heap creates a pthread key that tracks threads that use this heap. This isn't necessary for workers, which are tied to a single thread.
Created attachment 25307 [details]
Comment on attachment 25307 [details]
> #ifndef NDEBUG
> + int error =
> + pthread_key_delete(m_currentThreadRegistrar);
> + ASSERT(!error);
> + }
The little ditties we do so we can assert the results of these functions are ugly enough that I think we should instead, in the future, make little helper inline functions to encapsulate them. They distract too much from the rest of the flow of the functions they are in.
> +void Heap::makeUsableFromMultipleThreads()
> + if (m_currentThreadRegistrar)
> + return;
> + int error = pthread_key_create(&m_currentThreadRegistrar, unregisterThread);
> + if (error)
> + CRASH();
I wonder what we should do about cases like this. I think I understand what's going on -- this is an assertion, but one so important that you'd like it to be included in production code. But I'm not sure that would be clear to other readers. Also, it's not clear when to do abort() and when to do CRASH(). We should create some guidelines for this.
> +#if ENABLE(JSC_MULTIPLE_THREADS)
> + instance->makeUsableFromMultipleThreads();
In general, when the semantics of a function like this is simple, I prefer to make empty inline versions so we don't need to make each call site conditional. That would have prevented us from needing conditionals in JSGlobalData.h and here in JSGlobalData.cpp and in JSContextRef.cpp.
What do you think of that practice?
r=me as is
Committed revision 38622.
(In reply to comment #2)
> Also, it's not clear when to do abort() and when to do CRASH(). We
> should create some guidelines for this.
I don't like calling abort(), because it doesn't give any information for a bug report. Also, I think it's impolite to silently close an application. as if it was never running.
> What do you think of that practice?
In general, it's fine, but I didn't want code that's not guarded with ENABLE(JSC_MULTIPLE_THREADS) to call makeUsableFromMultipleThreads(), and I still don't want to.