Re: [webkit-dev] A catalog of iOS-specific changes related to the WebThread
Thanks for the heads up! -Darin On Sun, Feb 10, 2013 at 1:53 PM, Maciej Stachowiak wrote: > > Here is a list all iOS-specific changes in WebCore and below that are due > to the iOS WebKit threading model, from an exhaustive review of diffs to > the downstream source. I'm not sure anyone needs this to make a decision > any more, but it may be helpful to people to know what's coming as we > merge, and to provide input about specific items if they care to. > > > Key: > - Probably has to be upstreamed for the WebThread to work in the open > source tree > = Probably has to be upstreamed, but only affects files specific to the > iOS port and/or the Mac port > + Can likely be removed before upstreaming > % Can probably be removed sooner than other differences (these things are > needed for a non-WebKit use on the system which is likely to go away) > ? I'm not sure what status this is > > > In WTF: > - Changing WTF::MainThread to recognize either the web thread of the main > thread > - Changes to ThreadRestrictionVerifier to deal with web thread vs main > thread > - Changing the assert in StringStatics.cpp to assert the true main thread, > not isMainThread() (probably not strictly necessary) > - Change the StackBounds consistency check to adapt to main thread vs Web > thread (this is a class used by JSC for conservative stack scanning) > - Sharing of the JSC identifier table (but no locking for that) > % Locking and sharing for AtomicString (possibly removable; was added to > avoid a crash on exit) > > In JSC, one change total: > - A trick to avoid creating the GC timer on the wrong thread > > In WebCore: > - In JSDOMWindowBase.cpp, arrange for JSC to handle the Web thread > correctly > - A change to get ThreadGlobalData to be the same on the web thread and > the main thread > - ScriptDebugServer.cpp, drop and reacquire the web thread lock around a > nested run loop in ScriptDebugServer > - The Web thread messages the main thread about some events via > WKContentObservation > = The actual implementation of the Web Thread and associated locking and > messaging mechanisms > = Not initializing threading in SharedBufferMac.mm +initialize (to avoid > initializing on the wrong thread) > = Not initializing threading in WebScriptObject.mm +initialize (to avoid > initializing on the wrong thread) > = The iOS implementation of SharedTimer operates on the WebThread run loop > and messages the WebThread > = Taking the Web thread lock in an iOS-specific accessibility > implementation file > = Locking around the ObjC DOM wrapper cache > = Schedule CFNetwork callbacks on the Web thread runloop rather than the > main runloop > = The iOS tile cache implementation uses locking and messaging between the > web thread and main thread much like the tile cache in the open source tree > does it between the main thread and the scrolling thread > = CALayer implementations sometimes grab the global lock and/or message to > the WebThread from the main thread (affects WebLayer, PlatformCALayer, and > a special layer that exists for text tracks) > = An iBooks-specific workaround to target the main thread in > LayerFlushSchedulerMac > = Some messaging from the web thread to the main thread due to the weird > way HTML5 video is implemented on iOS, in CA-specific files > = WebCoreMotionManager (a new file that does not exist upstream) uses > WebThread primitives > + Some places that check isMainThread() || pthread_main_np(), which I > expect will not be needed with the isMainThread() change and will likely be > fixed before upstreaming; we're changing isMainThread() specifically to > avoid sprinkling these diffs all over WebCore > + In several places, avoid asserting m_thread == currentThread() (these > diffs can probably be eliminated as by introducing an isCurrentThread() > function) > + An iOS-specific extra ASSERT in ThreadTimers.cpp (probably not needed) > + Apparently obsolete includes of "WebCoreThread.h" in some files (e.g. > TextBreakIteratorICU.cpp) > + Apparently obsolete code to include "WebCoreThreadMessage.h" in some > generated bindings (seems unused) > + Apparently obsolete code to include "WebCoreThreadMessage.h" in some > files (e.g. DOMHTML.mm) > + An iOS-specific DiskImageCache in WebCore/loader uses threading in a > non-portable way - it directly uses libdispatch and messages the WebThread. > We can likely either remove it or rewrite it to do its threading in a more > portable way. > % A mutex in FontCache.cpp around access to the font cache (needed for a > non-WebKit use of WebCore that's likely to be phased out a fair bit sooner > than the Web thread) > ? Make ScriptController::initializeThreading a no-op (no idea why this is > needed) > ? Make HTMLParserScheduler yield if it is at a yield candidate point and > the main thread is waiting on the web thread lock (this was done long ago > for responsiveness, but we need to retest) > ? A mutex to guard creation and pausing of the database thread (I don't > understand why th
Re: [webkit-dev] A catalog of iOS-specific changes related to the WebThread
On Feb 10, 2013, at 5:14 PM, Brent Fulgham wrote: > Hi Maciej, > > Thank you for taking the time to clearly enumerate these different tasks. It > must have been a tedious effort, but I think the list is quite helpful in > understanding the changes you are proposing. > > 1. The WTF changes seem like they could be easily protected by preprocessor > macros to not impact other ports. > 2. The WebCore ThreadGlobalData change sounds like it could be easily > isolated at compile time to the iOS port. > 3. Most of the WebCore changes involved CFNetwork/CALayer logic, neither of > which (I believe) is used by the Chromium port. To clarify, essentially all of the changes are already in ifdefs in our downstream tree, and will remain so. There is no compile time impact on other ports. The main worry is that supporting this weird threading model could be a maintainability headache for the cross-platform code. The reality is that it will be some headache, but hopefully not too much. > > It sounds like the port most likely to be impacted by these changes would be > Apple's own Mac port. > > Reading this list, I'm also curious why we would ever want to call > "pthread_main_np", given the existence of isMainThread. If we were > consistent about this, the iOS "one of two 'main' threads" stuff would be > safely encapsulated. > > I like the idea of getting rid of the explicit "m_thread == currentThread()" > asserts, in favor of some form of encapsulated predicate. Yeah, I think the above two categories of changes can be fixed before merging and won't result in any diffs, ultimately. > > All in all, I think these changes seem unlikely to be harmful, and will > probably prompt some useful refactoring to clean a few things up. Thanks for reviewing the list. Cheers, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] A catalog of iOS-specific changes related to the WebThread
Hi Maciej, Thank you for taking the time to clearly enumerate these different tasks. It must have been a tedious effort, but I think the list is quite helpful in understanding the changes you are proposing. 1. The WTF changes seem like they could be easily protected by preprocessor macros to not impact other ports. 2. The WebCore ThreadGlobalData change sounds like it could be easily isolated at compile time to the iOS port. 3. Most of the WebCore changes involved CFNetwork/CALayer logic, neither of which (I believe) is used by the Chromium port. It sounds like the port most likely to be impacted by these changes would be Apple's own Mac port. Reading this list, I'm also curious why we would ever want to call "pthread_main_np", given the existence of isMainThread. If we were consistent about this, the iOS "one of two 'main' threads" stuff would be safely encapsulated. I like the idea of getting rid of the explicit "m_thread == currentThread()" asserts, in favor of some form of encapsulated predicate. All in all, I think these changes seem unlikely to be harmful, and will probably prompt some useful refactoring to clean a few things up. -Brent ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] A catalog of iOS-specific changes related to the WebThread
Here is a list all iOS-specific changes in WebCore and below that are due to the iOS WebKit threading model, from an exhaustive review of diffs to the downstream source. I'm not sure anyone needs this to make a decision any more, but it may be helpful to people to know what's coming as we merge, and to provide input about specific items if they care to. Key: - Probably has to be upstreamed for the WebThread to work in the open source tree = Probably has to be upstreamed, but only affects files specific to the iOS port and/or the Mac port + Can likely be removed before upstreaming % Can probably be removed sooner than other differences (these things are needed for a non-WebKit use on the system which is likely to go away) ? I'm not sure what status this is In WTF: - Changing WTF::MainThread to recognize either the web thread of the main thread - Changes to ThreadRestrictionVerifier to deal with web thread vs main thread - Changing the assert in StringStatics.cpp to assert the true main thread, not isMainThread() (probably not strictly necessary) - Change the StackBounds consistency check to adapt to main thread vs Web thread (this is a class used by JSC for conservative stack scanning) - Sharing of the JSC identifier table (but no locking for that) % Locking and sharing for AtomicString (possibly removable; was added to avoid a crash on exit) In JSC, one change total: - A trick to avoid creating the GC timer on the wrong thread In WebCore: - In JSDOMWindowBase.cpp, arrange for JSC to handle the Web thread correctly - A change to get ThreadGlobalData to be the same on the web thread and the main thread - ScriptDebugServer.cpp, drop and reacquire the web thread lock around a nested run loop in ScriptDebugServer - The Web thread messages the main thread about some events via WKContentObservation = The actual implementation of the Web Thread and associated locking and messaging mechanisms = Not initializing threading in SharedBufferMac.mm +initialize (to avoid initializing on the wrong thread) = Not initializing threading in WebScriptObject.mm +initialize (to avoid initializing on the wrong thread) = The iOS implementation of SharedTimer operates on the WebThread run loop and messages the WebThread = Taking the Web thread lock in an iOS-specific accessibility implementation file = Locking around the ObjC DOM wrapper cache = Schedule CFNetwork callbacks on the Web thread runloop rather than the main runloop = The iOS tile cache implementation uses locking and messaging between the web thread and main thread much like the tile cache in the open source tree does it between the main thread and the scrolling thread = CALayer implementations sometimes grab the global lock and/or message to the WebThread from the main thread (affects WebLayer, PlatformCALayer, and a special layer that exists for text tracks) = An iBooks-specific workaround to target the main thread in LayerFlushSchedulerMac = Some messaging from the web thread to the main thread due to the weird way HTML5 video is implemented on iOS, in CA-specific files = WebCoreMotionManager (a new file that does not exist upstream) uses WebThread primitives + Some places that check isMainThread() || pthread_main_np(), which I expect will not be needed with the isMainThread() change and will likely be fixed before upstreaming; we're changing isMainThread() specifically to avoid sprinkling these diffs all over WebCore + In several places, avoid asserting m_thread == currentThread() (these diffs can probably be eliminated as by introducing an isCurrentThread() function) + An iOS-specific extra ASSERT in ThreadTimers.cpp (probably not needed) + Apparently obsolete includes of "WebCoreThread.h" in some files (e.g. TextBreakIteratorICU.cpp) + Apparently obsolete code to include "WebCoreThreadMessage.h" in some generated bindings (seems unused) + Apparently obsolete code to include "WebCoreThreadMessage.h" in some files (e.g. DOMHTML.mm) + An iOS-specific DiskImageCache in WebCore/loader uses threading in a non-portable way - it directly uses libdispatch and messages the WebThread. We can likely either remove it or rewrite it to do its threading in a more portable way. % A mutex in FontCache.cpp around access to the font cache (needed for a non-WebKit use of WebCore that's likely to be phased out a fair bit sooner than the Web thread) ? Make ScriptController::initializeThreading a no-op (no idea why this is needed) ? Make HTMLParserScheduler yield if it is at a yield candidate point and the main thread is waiting on the web thread lock (this was done long ago for responsiveness, but we need to retest) ? A mutex to guard creation and pausing of the database thread (I don't understand why this is needed) ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
