Renamed the functions and added better comments.
Reduced the public API footprint by removing the IsProfiling() API.  It
was a little ambiguous (what exactly is 'profiling'), and I wasn't using
it.
The publicly exposed method is now called "EnableProfilerSampling",
which is a bit of a yucky name.  I am thinking about splitting into two
functions - PauseProfiler() and ResumeProfiler().  Let me know if you
have an opinion.


http://codereview.chromium.org/19607/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/19607/diff/1/2#newcode1997
Line 1997: static bool IsProfiling();
On 2009/01/28 01:22:56, iposva wrote:
> Some minimal comments, please. To at least somewhat document what
these calls
> are supposed to be doing.

Done.

http://codereview.chromium.org/19607/diff/1/3
File src/api.cc (right):

http://codereview.chromium.org/19607/diff/1/3#newcode2678
Line 2678: #endif
Good catch.
Removed this API since I wasn't using it anyway.

On 2009/01/28 01:22:56, iposva wrote:
> What do you return when ENABLE_LOGGING_AND_PROFILING is not set?
> Also we might want to add a way to determine if the profiler is
available at
> all. Not 100% sure about that second suggestion.

http://codereview.chromium.org/19607/diff/1/5
File src/log.cc (right):

http://codereview.chromium.org/19607/diff/1/5#newcode129
Line 129: static bool paused_;
On 2009/01/28 01:22:56, iposva wrote:
> Small comment, such as:
> When the profiler is paused, then incoming ticks will be ignored.

Done.

http://codereview.chromium.org/19607/diff/1/6
File src/log.h (right):

http://codereview.chromium.org/19607/diff/1/6#newcode209
Line 209: static void resume();
On 2009/01/28 01:22:56, iposva wrote:
> By looking at the use of "Logger::pause()" I assumed that all logging
is paused
> from here on, but you really only pause the profiler in the
implementation. My
> initial assumption from looking how you hooked this into the API was,
that I
> thought you are keeping the ticks coming and are smartly reusing some
existing
> way of just pausing the output, causing the ticks to not be put into
the log
> file.
>
> Should we change the name of this to be more explicit? For example:
> Logger::PauseProfiler()
>
>

Done.

http://codereview.chromium.org/19607

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to