First round of comments...
https://codereview.chromium.org/104583003/diff/1/src/libplatform/default-platform.cc
File src/libplatform/default-platform.cc (right):
https://codereview.chromium.org/104583003/diff/1/src/libplatform/default-platform.cc#newcode88
src/libplatform/default-platform.cc:88: thread_pool_size_ =
Max(Min(thread_pool_size_, 4), 1);
Wouldn't it be better to do some sanity checks/clamping in
DefaultPlatform::SetThreadPoolSize instead? Furhtermore, it might be
nice to have a flag for the hardcoded limit of 4.
https://codereview.chromium.org/104583003/diff/1/src/libplatform/default-platform.h
File src/libplatform/default-platform.h (right):
https://codereview.chromium.org/104583003/diff/1/src/libplatform/default-platform.h#newcode60
src/libplatform/default-platform.h:60: // TODO(jochen): This shouldn't
be a static.
What's the reason for "static"? Wouldn't it be easier to have a
std::list<WorkerThread*> here? Furthermore, the WorkerThreads are owned
by the list, so it might be nice to have a std::list<WorkerThread>, but
I guess we would have to do some massaging to the WorkerThread class
hierarchy to make the threads auto-join on destruction.
https://codereview.chromium.org/104583003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
