see patch
Created attachment 60766 [details] Patch
Attachment 60766 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/wtf/FastMalloc.cpp:418: Alphabetical sorting problem. [build/include_order] [4] JavaScriptCore/wtf/FastMalloc.cpp:1447: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] JavaScriptCore/wtf/FastMalloc.cpp:1448: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 60766 [details] Patch r=me
Comment on attachment 60766 [details] Patch Clearing flags on attachment: 60766 Committed r62765: <http://trac.webkit.org/changeset/62765>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/62765 might have broken Qt Linux Release
(In reply to comment #6) > http://trac.webkit.org/changeset/62765 might have broken Qt Linux Release Rolled out by http://trac.webkit.org/changeset/62768 because it broke all jscore and layout tests on Qt bot. http://build.webkit.org/builders/Qt%20Linux%20Release/builds/14904/steps/jscore-test/logs/stdio http://build.webkit.org/results/Qt%20Linux%20Release/r62767%20(14904) I cc-ed the fastmalloc perfect Zoltan. Could you check what was the problem?
If we use threading.h, and wtf based mutexes, we should ALSO create the thread by wtf based method. runScavengerThread calls the scavengerThread() 1492 void TCMalloc_PageHeap::initializeScavenger() 1493 { 1494 m_scavengeThreadActive = true; 1495 pthread_t thread; 1496 pthread_create(&thread, 0, runScavengerThread, this); 1497 } pthread_create - should be replaced by its threading.h variant I think Qt depends on some thread specific variables, which are not created for the thread by pthread_create I suggest to rollout this patch until we fix and test this issue
ThreadSpecific.h allocates memory using FastMalloc, so it can’t be used within FastMalloc’s implementation. It didn’t appear to be used by the attached patch anyway, so maybe that’s besides the point. I don’t think Threading.h is the right header to include. The data types used all seem to come from ThreadingPrimitives.h, so why not include that instead? I’m also wary of introducing WTF types in to FastMalloc. Many bits of code in WTF allocate memory in order to do their job, which isn’t something that’s sensible to do via the normal means from within the memory allocator. WTF::createThread is one such function. I’m curious why this weird subset of FastMalloc was modified in this manner. There’s a bunch of other code in FastMalloc that uses pthreads after all.
(In reply to comment #9) > ThreadSpecific.h allocates memory using FastMalloc, so it can’t be used within FastMalloc’s implementation. It didn’t appear to be used by the attached patch anyway, so maybe that’s besides the point. I need the following lines for WinCE and didn't wanted to copy them: #ifndef TLS_OUT_OF_INDEXES #define TLS_OUT_OF_INDEXES 0xffffffff #endif > I don’t think Threading.h is the right header to include. The data types used all seem to come from ThreadingPrimitives.h, so why not include that instead? > I’m also wary of introducing WTF types in to FastMalloc. Many bits of code in WTF allocate memory in order to do their job, which isn’t something that’s sensible to do via the normal means from within the memory allocator. WTF::createThread is one such function. > I’m curious why this weird subset of FastMalloc was modified in this manner. There’s a bunch of other code in FastMalloc that uses pthreads after all. I ported the other code too to native windows, but when using WTF::createThread there is exactily this problem: WTF::createThreadInternal (at least on windows) does a "new ThreadFunctionInvocation", which wants to be allocated via FastMalloc, and this results in an endless loop. Is there a good way to reuse the current Mutex/ThreadCondition/ThreadSpecific stuff? Or do I need to implement it again? Maybe we can only fix the Qt problem? What about QtWin? I don't think that allocating Mutex and ThreadCondition via Fastalloc should be a problem? Any ideas about the exact Qt problem?
There doesn’t appear to be any information in the bug about what the Qt problem *is*.
Created attachment 62683 [details] Some basics changes
Attachment 62683 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/FastMalloc.cpp:3065: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] JavaScriptCore/wtf/FastMalloc.cpp:3065: Use 0 instead of NULL. [readability/null] [5] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 62683 [details] Some basics changes The changes make sense, and look fine. But Why? Which non-MSVC windows compiler are you trying to compile with? I assume this is some sort of Qt build? (Your ChangeLog should ideally say.)
(In reply to comment #14) > (From update of attachment 62683 [details]) > The changes make sense, and look fine. But Why? Which non-MSVC windows compiler are you trying to compile with? I assume this is some sort of Qt build? (Your ChangeLog should ideally say.) I try to port the FastMalloc to WinCE. The change form COMPILER(MSVC)to OS(WINDOWS) doesn't have any effect. Only cleanup (so no real reason to mention it explicit in ChangeLog).
Comment on attachment 62683 [details] Some basics changes Clearing flags on attachment: 62683 Committed r65091: <http://trac.webkit.org/changeset/65091>