[webkit-reviews] review denied: [Bug 43903] Speeding up SVG filters with multicore (SMP) support : [Attachment 83307] Speeding up FETurbulence with SMP

[bugzilla-daemon at webkit.org]

Csaba Osztrogonac <ossy at webkit.org> has denied Gabor Loki <loki at webkit.org>'s
request for review:
Bug 43903: Speeding up SVG filters with multicore (SMP) support
https://bugs.webkit.org/show_bug.cgi?id=43903

Attachment 83307: Speeding up FETurbulence with SMP
https://bugs.webkit.org/attachment.cgi?id=83307&action=review

------- Additional Comments from Csaba Osztrogonac <ossy at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=83307&action=review

On the whole looks good to me, but please fix the following things:

- I agree with Zoltan ( https://bugs.webkit.org/show_bug.cgi?id=43903#c16 ), we
need 
template for parameter passing to avoid ugly reinterpret_casts and using sizof.


- Please fix the typo detected by smfr. (s/Simmetric/Symmetric)

- And what about the other build systems? You should add the new files
for all build system. ( Or at least disable PARALLEL_JOBS if !PLATFORM(QT) )
For example it doesn't build on Windows because of missing include path.

r- for the present due to my comments above and below.

> Source/JavaScriptCore/wtf/ParallelJobs.cpp:135
> +    ASSERT(m_numberOfJobs > 0);

m_numberOfJobs is always greater than 0 if !ENABLE(OPENMP)

> Source/JavaScriptCore/wtf/ParallelJobs.h:44
> +//	  ParallelJobs parallelJobs(&worker, sizeof(Parameters));

s/worker/workerFunction

> Source/JavaScriptCore/wtf/Platform.h:1162
> +#if !defined(ENABLE_PARALLEL_JOBS) && ENABLE(OPENMP)
> +#define ENABLE_PARALLEL_JOBS 1
> +#endif

We shouldn't enable parallel jobs if ENABLE(SINGLE_THREADED) is 
true, so we need an additional !ENABLE(SINGLE_THREADED) guard.