[webkit-reviews] review granted: [Bug 208223] Allow setting of stack sizes for threads : [Attachment 391762] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Feb 26 12:03:53 PST 2020
Mark Lam <mark.lam at apple.com> has granted Don Olmstead
<don.olmstead at sony.com>'s request for review:
Bug 208223: Allow setting of stack sizes for threads
https://bugs.webkit.org/show_bug.cgi?id=208223
Attachment 391762: Patch
https://bugs.webkit.org/attachment.cgi?id=391762&action=review
--- Comment #5 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 391762
--> https://bugs.webkit.org/attachment.cgi?id=391762
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=391762&action=review
r=me with fixes.
> Source/WTF/ChangeLog:11
> + based on their own observations. Additionally a default stack size
can be specified
> + during by defining DEFAULT_THREAD_STACK_SIZE_IN_KB if the default
stack for a platform is
Can you clarify what you mean here by "specified during"? I suspect you're
missing some details.
> Source/WTF/wtf/Threading.cpp:53
>
> +#if defined(DEFAULT_THREAD_STACK_SIZE_IN_KB) &&
DEFAULT_THREAD_STACK_SIZE_IN_KB > 0
> +static constexpr size_t defaultStackSize = DEFAULT_THREAD_STACK_SIZE_IN_KB *
1024;
> +#else
> +// Use the platform's default stack size
> +static constexpr size_t defaultStackSize = 0;
> +#endif
Just move this into Thread::create() and initialize an Optional<size_t> there.
> Source/WTF/wtf/Threading.cpp:162
> + // If the stack size is set to 0 then use the system default. Platforms
can tune the values here.
Please add a comma after '0'.
> Source/WTF/wtf/Threading.h:247
> + bool establishHandle(NewThreadContext*, size_t);
Please use Optional<size_t> instead of size_t. Then we don't have to worry
about the convention of a size of 0 meaning something special.
> Source/WTF/wtf/posix/ThreadingPOSIX.cpp:204
> +bool Thread::establishHandle(NewThreadContext* context, size_t stackSize)
Use Optional<size_t>.
> Source/WTF/wtf/win/ThreadingWin.cpp:156
> +bool Thread::establishHandle(NewThreadContext* data, size_t stackSize)
Use Optional<size_t>.
More information about the webkit-reviews
mailing list