On 2015/03/23 at 10:13:27, svenpanne wrote:
I'm a bit unhappy about the complicated ownership/lifetime handling in
this
CL. It looks like premature optimization in an already horribly complicated
and
bloated class (CompilationInfo). Unless we have hard measured numbers to
justify
this, we should keep things as simple as possible, because heavy
refactorings in
this area are coming our way.
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.cc#newcode134
src/compiler.cc:134: no_frame_ranges_.Reset(new List<OffsetRange>(2));
No need for this, see comment in the header.
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.cc#newcode135
src/compiler.cc:135: track_positions_ = true;
Instead of doing this assignment to track_positions, just do it in the
initializer list.
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.cc#newcode138
src/compiler.cc:138: inlined_function_infos_.Reset(new
std::vector<InlinedFunctionInfo>());
No need for this, see comment in the header.
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.h
File src/compiler.h (right):
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.h#newcode352
src/compiler.h:352: return inlined_function_infos_.Detach();
This is horribly complicated: Can't we simply return the
inlined_function_infos_ by value? How many entries are there typically in
such a
vector? AFAICT, there should be only a handful.
The same holds for the Detach above, too.
https://codereview.chromium.org/1013143003/diff/150001/src/compiler.h#newcode465
src/compiler.h:465: SmartPointer<std::vector<InlinedFunctionInfo>>
inlined_function_infos_;
Following the KISS principle, leave the instance fields just like they
were
without any explicit dynamic allocation/SmartPointers/etc. Don't
nano-optimize
the CompilationInfo memory consumption when we burn tons of MBs later in the
compilation pipelines, anyway.
comments addressed
https://codereview.chromium.org/1013143003/
--
--
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/d/optout.
