Skip to content

Commit 4a2e91b

Browse files
psmarshallCommit Bot
psmarshall
authored and
Commit Bot
committed
[cleanup] Remove unused TickSample class from the public API
We have internal::TickSample which inherits from this, but we never use the public version in the API despite defining it there. Change-Id: I6f0ce7ee663ef821be57cfbad540c1660484a525 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1745472 Commit-Queue: Peter Marshall <petermarshall@chromium.org> Reviewed-by: Alexei Filippov <alph@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#63329}
1 parent d0e718a commit 4a2e91b

10 files changed

Lines changed: 100 additions & 123 deletions

File tree

‎include/v8-profiler.h‎

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -52,82 +52,6 @@ template class V8_EXPORT std::vector<v8::CpuProfileDeoptInfo>;
5252

5353
namespace v8 {
5454

55-
// TickSample captures the information collected for each sample.
56-
struct V8_EXPORT TickSample {
57-
// Internal profiling (with --prof + tools/$OS-tick-processor) wants to
58-
// include the runtime function we're calling. Externally exposed tick
59-
// samples don't care.
60-
enum RecordCEntryFrame { kIncludeCEntryFrame, kSkipCEntryFrame };
61-
62-
TickSample()
63-
: state(OTHER),
64-
pc(nullptr),
65-
external_callback_entry(nullptr),
66-
frames_count(0),
67-
has_external_callback(false),
68-
update_stats(true) {}
69-
70-
/**
71-
* Initialize a tick sample from the isolate.
72-
* \param isolate The isolate.
73-
* \param state Execution state.
74-
* \param record_c_entry_frame Include or skip the runtime function.
75-
* \param update_stats Whether update the sample to the aggregated stats.
76-
* \param use_simulator_reg_state When set to true and V8 is running under a
77-
* simulator, the method will use the simulator
78-
* register state rather than the one provided
79-
* with |state| argument. Otherwise the method
80-
* will use provided register |state| as is.
81-
*/
82-
void Init(Isolate* isolate, const v8::RegisterState& state,
83-
RecordCEntryFrame record_c_entry_frame, bool update_stats,
84-
bool use_simulator_reg_state = true);
85-
/**
86-
* Get a call stack sample from the isolate.
87-
* \param isolate The isolate.
88-
* \param state Register state.
89-
* \param record_c_entry_frame Include or skip the runtime function.
90-
* \param frames Caller allocated buffer to store stack frames.
91-
* \param frames_limit Maximum number of frames to capture. The buffer must
92-
* be large enough to hold the number of frames.
93-
* \param sample_info The sample info is filled up by the function
94-
* provides number of actual captured stack frames and
95-
* the current VM state.
96-
* \param use_simulator_reg_state When set to true and V8 is running under a
97-
* simulator, the method will use the simulator
98-
* register state rather than the one provided
99-
* with |state| argument. Otherwise the method
100-
* will use provided register |state| as is.
101-
* \param contexts If set, contexts[i] will be set to the address of the
102-
* incumbent native context associated with frames[i]. It
103-
* should be large enough to hold |frames_limit| frame
104-
* contexts.
105-
* \note GetStackSample is thread and signal safe and should only be called
106-
* when the JS thread is paused or interrupted.
107-
* Otherwise the behavior is undefined.
108-
*/
109-
static bool GetStackSample(Isolate* isolate, v8::RegisterState* state,
110-
RecordCEntryFrame record_c_entry_frame,
111-
void** frames, size_t frames_limit,
112-
v8::SampleInfo* sample_info,
113-
bool use_simulator_reg_state = true,
114-
void** contexts = nullptr);
115-
StateTag state; // The state of the VM.
116-
void* pc; // Instruction pointer.
117-
union {
118-
void* tos; // Top stack value (*sp).
119-
void* external_callback_entry;
120-
};
121-
static const unsigned kMaxFramesCountLog2 = 8;
122-
static const unsigned kMaxFramesCount = (1 << kMaxFramesCountLog2) - 1;
123-
void* stack[kMaxFramesCount]; // Call stack.
124-
void* contexts[kMaxFramesCount]; // Stack of associated native contexts.
125-
void* top_context = nullptr; // Address of the incumbent native context.
126-
unsigned frames_count : kMaxFramesCountLog2; // Number of captured frames.
127-
bool has_external_callback : 1;
128-
bool update_stats : 1; // Whether the sample should update aggregated stats.
129-
};
130-
13155
/**
13256
* CpuProfileNode represents a node in a call graph.
13357
*/

‎src/api/api.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8199,8 +8199,10 @@ bool Isolate::GetHeapCodeAndMetadataStatistics(
81998199
void Isolate::GetStackSample(const RegisterState& state, void** frames,
82008200
size_t frames_limit, SampleInfo* sample_info) {
82018201
RegisterState regs = state;
8202-
if (TickSample::GetStackSample(this, &regs, TickSample::kSkipCEntryFrame,
8203-
frames, frames_limit, sample_info)) {
8202+
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
8203+
if (i::TickSample::GetStackSample(isolate, &regs,
8204+
i::TickSample::kSkipCEntryFrame, frames,
8205+
frames_limit, sample_info)) {
82048206
return;
82058207
}
82068208
sample_info->frames_count = 0;

‎src/logging/log.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ class Profiler : public base::Thread {
765765
void Disengage();
766766

767767
// Inserts collected profiling data into buffer.
768-
void Insert(v8::TickSample* sample) {
768+
void Insert(TickSample* sample) {
769769
if (Succ(head_) == static_cast<int>(base::Relaxed_Load(&tail_))) {
770770
overflow_ = true;
771771
} else {
@@ -779,7 +779,7 @@ class Profiler : public base::Thread {
779779

780780
private:
781781
// Waits for a signal and removes profiling data.
782-
bool Remove(v8::TickSample* sample) {
782+
bool Remove(TickSample* sample) {
783783
buffer_semaphore_.Wait(); // Wait for an element.
784784
*sample = buffer_[base::Relaxed_Load(&tail_)];
785785
bool result = overflow_;
@@ -796,7 +796,7 @@ class Profiler : public base::Thread {
796796
// Cyclic buffer for communicating profiling samples
797797
// between the signal handler and the worker thread.
798798
static const int kBufferSize = 128;
799-
v8::TickSample buffer_[kBufferSize]; // Buffer storage.
799+
TickSample buffer_[kBufferSize]; // Buffer storage.
800800
int head_; // Index to the buffer head.
801801
base::Atomic32 tail_; // Index to the buffer tail.
802802
bool overflow_; // Tell whether a buffer overflow has occurred.
@@ -888,15 +888,15 @@ void Profiler::Disengage() {
888888
// inserting a fake element in the queue and then wait for
889889
// the thread to terminate.
890890
base::Relaxed_Store(&running_, 0);
891-
v8::TickSample sample;
891+
TickSample sample;
892892
Insert(&sample);
893893
Join();
894894

895895
LOG(isolate_, UncheckedStringEvent("profiler", "end"));
896896
}
897897

898898
void Profiler::Run() {
899-
v8::TickSample sample;
899+
TickSample sample;
900900
bool overflow = Remove(&sample);
901901
while (base::Relaxed_Load(&running_)) {
902902
LOG(isolate_, TickEvent(&sample, overflow));
@@ -1549,7 +1549,7 @@ void Logger::RuntimeCallTimerEvent() {
15491549
msg.WriteToLogFile();
15501550
}
15511551

1552-
void Logger::TickEvent(v8::TickSample* sample, bool overflow) {
1552+
void Logger::TickEvent(TickSample* sample, bool overflow) {
15531553
if (!log_->IsEnabled() || !FLAG_prof_cpp) return;
15541554
if (V8_UNLIKELY(TracingFlags::runtime_stats.load(std::memory_order_relaxed) ==
15551555
v8::tracing::TracingCategoryObserver::ENABLED_BY_NATIVE)) {

‎src/logging/log.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@
1515

1616
namespace v8 {
1717

18-
struct TickSample;
19-
2018
namespace sampler {
2119
class Sampler;
2220
}
2321

2422
namespace internal {
2523

24+
struct TickSample;
25+
2626
// Logger is used for collecting logging information from V8 during
2727
// execution. The result is dumped to a file.
2828
//

‎src/profiler/tick-sample.cc‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "src/sanitizer/msan.h"
1717

1818
namespace v8 {
19+
namespace internal {
1920
namespace {
2021

2122
bool IsSamePage(i::Address ptr1, i::Address ptr2) {
@@ -78,11 +79,6 @@ bool IsNoFrameRegion(i::Address address) {
7879
return false;
7980
}
8081

81-
} // namespace
82-
83-
namespace internal {
84-
namespace {
85-
8682
#if defined(USE_SIMULATOR)
8783
class SimulatorHelper {
8884
public:
@@ -184,16 +180,13 @@ Address ScrapeNativeContextAddress(Heap* heap, Address context_address) {
184180
}
185181

186182
} // namespace
187-
} // namespace internal
188183

189-
//
190-
// StackTracer implementation
191-
//
192184
DISABLE_ASAN void TickSample::Init(Isolate* v8_isolate,
193185
const RegisterState& reg_state,
194186
RecordCEntryFrame record_c_entry_frame,
195187
bool update_stats,
196-
bool use_simulator_reg_state) {
188+
bool use_simulator_reg_state,
189+
base::TimeDelta sampling_interval) {
197190
this->update_stats = update_stats;
198191
SampleInfo info;
199192
RegisterState regs = reg_state;
@@ -229,6 +222,8 @@ DISABLE_ASAN void TickSample::Init(Isolate* v8_isolate,
229222
} else {
230223
tos = nullptr;
231224
}
225+
this->sampling_interval = sampling_interval;
226+
timestamp = base::TimeTicks::HighResolutionNow();
232227
}
233228

234229
bool TickSample::GetStackSample(Isolate* v8_isolate, RegisterState* regs,
@@ -368,20 +363,6 @@ bool TickSample::GetStackSample(Isolate* v8_isolate, RegisterState* regs,
368363
return true;
369364
}
370365

371-
namespace internal {
372-
373-
void TickSample::Init(Isolate* isolate, const v8::RegisterState& state,
374-
RecordCEntryFrame record_c_entry_frame, bool update_stats,
375-
bool use_simulator_reg_state,
376-
base::TimeDelta sampling_interval) {
377-
v8::TickSample::Init(reinterpret_cast<v8::Isolate*>(isolate), state,
378-
record_c_entry_frame, update_stats,
379-
use_simulator_reg_state);
380-
this->sampling_interval = sampling_interval;
381-
if (pc == nullptr) return;
382-
timestamp = base::TimeTicks::HighResolutionNow();
383-
}
384-
385366
void TickSample::print() const {
386367
PrintF("TickSample: at %p\n", this);
387368
PrintF(" - state: %s\n", StateToString(state));

‎src/profiler/tick-sample.h‎

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#ifndef V8_PROFILER_TICK_SAMPLE_H_
66
#define V8_PROFILER_TICK_SAMPLE_H_
77

8-
#include "include/v8-profiler.h"
8+
#include "include/v8.h"
99
#include "src/base/platform/time.h"
1010
#include "src/common/globals.h"
1111

@@ -14,15 +14,83 @@ namespace internal {
1414

1515
class Isolate;
1616

17-
struct TickSample : public v8::TickSample {
17+
// TickSample captures the information collected for each sample.
18+
struct V8_EXPORT TickSample {
19+
// Internal profiling (with --prof + tools/$OS-tick-processor) wants to
20+
// include the runtime function we're calling. Externally exposed tick
21+
// samples don't care.
22+
enum RecordCEntryFrame { kIncludeCEntryFrame, kSkipCEntryFrame };
23+
24+
TickSample()
25+
: state(OTHER),
26+
pc(nullptr),
27+
external_callback_entry(nullptr),
28+
frames_count(0),
29+
has_external_callback(false),
30+
update_stats(true) {}
31+
32+
/**
33+
* Initialize a tick sample from the isolate.
34+
* \param isolate The isolate.
35+
* \param state Execution state.
36+
* \param record_c_entry_frame Include or skip the runtime function.
37+
* \param update_stats Whether update the sample to the aggregated stats.
38+
* \param use_simulator_reg_state When set to true and V8 is running under a
39+
* simulator, the method will use the simulator
40+
* register state rather than the one provided
41+
* with |state| argument. Otherwise the method
42+
* will use provided register |state| as is.
43+
*/
1844
void Init(Isolate* isolate, const v8::RegisterState& state,
1945
RecordCEntryFrame record_c_entry_frame, bool update_stats,
2046
bool use_simulator_reg_state = true,
2147
base::TimeDelta sampling_interval = base::TimeDelta());
22-
base::TimeTicks timestamp;
23-
base::TimeDelta sampling_interval; // Sampling interval used to capture.
48+
/**
49+
* Get a call stack sample from the isolate.
50+
* \param isolate The isolate.
51+
* \param state Register state.
52+
* \param record_c_entry_frame Include or skip the runtime function.
53+
* \param frames Caller allocated buffer to store stack frames.
54+
* \param frames_limit Maximum number of frames to capture. The buffer must
55+
* be large enough to hold the number of frames.
56+
* \param sample_info The sample info is filled up by the function
57+
* provides number of actual captured stack frames and
58+
* the current VM state.
59+
* \param use_simulator_reg_state When set to true and V8 is running under a
60+
* simulator, the method will use the simulator
61+
* register state rather than the one provided
62+
* with |state| argument. Otherwise the method
63+
* will use provided register |state| as is.
64+
* \note GetStackSample is thread and signal safe and should only be called
65+
* when the JS thread is paused or interrupted.
66+
* Otherwise the behavior is undefined.
67+
*/
68+
static bool GetStackSample(Isolate* isolate, v8::RegisterState* state,
69+
RecordCEntryFrame record_c_entry_frame,
70+
void** frames, size_t frames_limit,
71+
v8::SampleInfo* sample_info,
72+
bool use_simulator_reg_state = true,
73+
void** contexts = nullptr);
2474

2575
void print() const;
76+
77+
StateTag state; // The state of the VM.
78+
void* pc; // Instruction pointer.
79+
union {
80+
void* tos; // Top stack value (*sp).
81+
void* external_callback_entry;
82+
};
83+
static const unsigned kMaxFramesCountLog2 = 8;
84+
static const unsigned kMaxFramesCount = (1 << kMaxFramesCountLog2) - 1;
85+
void* stack[kMaxFramesCount]; // Call stack.
86+
void* contexts[kMaxFramesCount]; // Stack of associated native contexts.
87+
void* top_context = nullptr; // Address of the incumbent native context.
88+
unsigned frames_count : kMaxFramesCountLog2; // Number of captured frames.
89+
bool has_external_callback : 1;
90+
bool update_stats : 1; // Whether the sample should update aggregated stats.
91+
92+
base::TimeTicks timestamp;
93+
base::TimeDelta sampling_interval; // Sampling interval used to capture.
2694
};
2795

2896
} // namespace internal

‎test/cctest/test-cpu-profiler.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ TEST(Issue1398) {
323323
v8::internal::TickSample sample;
324324
sample.pc = reinterpret_cast<void*>(code.InstructionStart());
325325
sample.tos = nullptr;
326-
sample.frames_count = v8::TickSample::kMaxFramesCount;
326+
sample.frames_count = TickSample::kMaxFramesCount;
327327
for (unsigned i = 0; i < sample.frames_count; ++i) {
328328
sample.stack[i] = reinterpret_cast<void*>(code.InstructionStart());
329329
}
@@ -341,7 +341,7 @@ TEST(Issue1398) {
341341
++actual_depth;
342342
}
343343

344-
CHECK_EQ(1 + v8::TickSample::kMaxFramesCount, actual_depth); // +1 for PC.
344+
CHECK_EQ(1 + TickSample::kMaxFramesCount, actual_depth); // +1 for PC.
345345
}
346346

347347
TEST(DeleteAllCpuProfiles) {

‎test/cctest/test-log-stack-tracer.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "src/execution/vm-state-inl.h"
3838
#include "src/init/v8.h"
3939
#include "src/objects/objects-inl.h"
40+
#include "src/profiler/tick-sample.h"
4041
#include "test/cctest/cctest.h"
4142
#include "test/cctest/trace-extension.h"
4243

‎test/cctest/trace-extension.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "include/v8-profiler.h"
3131
#include "src/execution/vm-state-inl.h"
3232
#include "src/objects/smi.h"
33+
#include "src/profiler/tick-sample.h"
3334
#include "test/cctest/cctest.h"
3435

3536
namespace v8 {
@@ -89,21 +90,20 @@ Address TraceExtension::GetFP(const v8::FunctionCallbackInfo<v8::Value>& args) {
8990
return fp;
9091
}
9192

92-
static struct { v8::TickSample* sample; } trace_env = {nullptr};
93+
static struct { TickSample* sample; } trace_env = {nullptr};
9394

94-
void TraceExtension::InitTraceEnv(v8::TickSample* sample) {
95+
void TraceExtension::InitTraceEnv(TickSample* sample) {
9596
trace_env.sample = sample;
9697
}
9798

98-
9999
void TraceExtension::DoTrace(Address fp) {
100100
RegisterState regs;
101101
regs.fp = reinterpret_cast<void*>(fp);
102102
// sp is only used to define stack high bound
103103
regs.sp = reinterpret_cast<void*>(
104104
reinterpret_cast<Address>(trace_env.sample) - 10240);
105-
trace_env.sample->Init(CcTest::isolate(), regs,
106-
v8::TickSample::kSkipCEntryFrame, true);
105+
trace_env.sample->Init(CcTest::i_isolate(), regs,
106+
TickSample::kSkipCEntryFrame, true);
107107
}
108108

109109

0 commit comments

Comments
 (0)