[Webkit-unassigned] [Bug 73309] [Qt] GC should be parallel on Qt platform

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73309





--- Comment #3 from Gabor Loki <loki at webkit.org>  2011-11-30 02:07:37 PST ---
(From update of attachment 117142)
View in context: https://bugs.webkit.org/attachment.cgi?id=117142&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        These changes made the parallel gc feature available for non-mac ports.

That is not true. This patch adds Qt support only.
BTW, you have moved the core detection routine from ParallelJobs to WTF, which is OK, but you have forgot to mention this move here.

> Source/JavaScriptCore/ChangeLog:17
> +        * wtf/NumberOfCores.cpp: Added.

Did you add this to the project files? It looks like you missed it.

> Source/JavaScriptCore/ChangeLog:19
> +        * wtf/NumberOfCores.h: Added.

Ditto.

> Source/JavaScriptCore/runtime/Heuristics.cpp:45
> +#include <wtf/NumberOfCores.h>

I guess it will be an alphabetical order issue.

> Source/JavaScriptCore/wtf/NumberOfCores.cpp:61
> +        if (sysctlResult >= 0) {
> +
> +            s_numOfCores = result;
> +
> +        }

Empty lines!
No need for the curly brackets in this case!

> Source/JavaScriptCore/wtf/NumberOfCores.cpp:71
> +        if (sysconfResult >= 0) {
> +
> +            s_numOfCores = static_cast<int>(sysconfResult);
> +
> +        }

Empty lines, curly brackets!



Well, I guess it will fail on several ports. You should consider the project files.
Before sending a patch for review you should make sure do not brake anything.
At least you have to execute run-webkit-test on Qt port.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.