Commit 15a78f97 authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

Revert "Reland "[cpu-profiler] Log OSR code when starting the profiler""

This reverts commit 8b60d8fc.

Reason for revert: Flaky on windows: https://ci.chromium.org/p/v8/builders/ci/V8%20Win32%20-%20debug/27302

Original change's description:
> Reland "[cpu-profiler] Log OSR code when starting the profiler"
> 
> This is a reland of f6965281
> 
> Updated the test:
> 1. Set profiling interval to 100us to get 10x the samples
> 2. Guarantee we spend at least 1ms per iteration, instead of only
> bailing out if we spend more than 1ms. This gives us enough samples on
> release mode.
> 3. Increase the time spent profiling optimized code by 50% to make sure
> we have a big enough difference.
> 
> With 1000 iterations I didn't see any flakes locally so this looks solid
> now.
> 
> Original change's description:
> > [cpu-profiler] Log OSR code when starting the profiler
> >
> > OSR code doesn't hang off any JSFunction or SFI, so we missed it when
> > starting up the profiler. This meant we didn't properly attribute
> > ticks to SFI code. The ticks ended up going to the caller instead.
> >
> > There is a weak cache of OSR code per native context, so iterate that
> > on profiler startup and log all the code objects.
> >
> > Change-Id: I2e9738b86a488b37f36ac89803561607dc76f745
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2414216
> > Commit-Queue: Peter Marshall <petermarshall@chromium.org>
> > Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> > Reviewed-by: Mythri Alle <mythria@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#69964}
> 
> Change-Id: Ib506e88b546008e462967259763bbf985b74b462
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2418092
> Commit-Queue: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Reviewed-by: Mythri Alle <mythria@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#69990}

TBR=mythria@chromium.org,petermarshall@chromium.org,dinfuehr@chromium.org

Change-Id: Ie3272c4fd297ca6f10a47c3fe8826e226a9f0545
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2418714Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69999}
parent 91c562ee
......@@ -1528,8 +1528,6 @@ class Heap {
static Isolate* GetIsolateFromWritableObject(HeapObject object);
std::vector<Handle<NativeContext>> FindAllNativeContexts();
private:
using ExternalStringTableUpdaterCallback = String (*)(Heap* heap,
FullObjectSlot pointer);
......@@ -2021,6 +2019,7 @@ class Heap {
V8_EXPORT_PRIVATE void IncrementObjectCounters();
#endif // DEBUG
std::vector<Handle<NativeContext>> FindAllNativeContexts();
std::vector<WeakArrayList> FindAllRetainedMaps();
MemoryMeasurement* memory_measurement() { return memory_measurement_.get(); }
......
......@@ -33,7 +33,6 @@
#include "src/logging/log-inl.h"
#include "src/logging/log-utils.h"
#include "src/objects/api-callbacks.h"
#include "src/objects/osr-optimized-code-cache.h"
#include "src/profiler/tick-sample.h"
#include "src/snapshot/embedded/embedded-data.h"
#include "src/strings/string-stream.h"
......@@ -1898,17 +1897,6 @@ static int EnumerateCompiledFunctions(Heap* heap,
}
}
for (Handle<NativeContext> native_context : heap->FindAllNativeContexts()) {
OSROptimizedCodeCache::Iterator osr_iterator(native_context);
for (std::pair<SharedFunctionInfo, Code> sfiCodePair = osr_iterator.Next();
!sfiCodePair.first.is_null(); sfiCodePair = osr_iterator.Next()) {
AddFunctionAndCode(sfiCodePair.first,
AbstractCode::cast(sfiCodePair.second), sfis,
code_objects, compiled_funcs_count);
++compiled_funcs_count;
}
}
return compiled_funcs_count;
}
......
......@@ -219,24 +219,5 @@ bool OSROptimizedCodeCache::NeedsTrimming(int num_valid_entries,
return curr_length > kInitialLength && curr_length > num_valid_entries * 3;
}
OSROptimizedCodeCache::Iterator::Iterator(Handle<NativeContext> native_context)
: cache_(native_context->GetOSROptimizedCodeCache(),
native_context->GetIsolate()) {}
std::pair<SharedFunctionInfo, Code> OSROptimizedCodeCache::Iterator::Next() {
while (index_ < cache_->length()) {
if (cache_->Get(index_ + kSharedOffset)->IsCleared() ||
cache_->Get(index_ + kCachedCodeOffset)->IsCleared()) {
index_ += kEntryLength;
continue;
}
auto result = std::make_pair(cache_->GetSFIFromEntry(index_),
cache_->GetCodeFromEntry(index_));
index_ += kEntryLength;
return result;
}
return std::make_pair(SharedFunctionInfo(), Code());
}
} // namespace internal
} // namespace v8
......@@ -48,19 +48,6 @@ class V8_EXPORT OSROptimizedCodeCache : public WeakFixedArray {
// Remove all code objects marked for deoptimization from OSR code cache.
void EvictMarkedCode(Isolate* isolate);
// Iterate over all entries in the cache, skipping cleared or null entries.
class Iterator {
public:
explicit Iterator(Handle<NativeContext> native_context);
// Both results will be null when iteration has finished.
std::pair<SharedFunctionInfo, Code> Next();
private:
Handle<OSROptimizedCodeCache> cache_;
int index_ = 0; // Index into the FixedArray; a multiple of kEntryLength.
};
private:
// Functions that implement heuristics on when to grow / shrink the cache.
static int CapacityForLength(int curr_capacity);
......
......@@ -2226,10 +2226,11 @@ static const char* pre_profiling_osr_script = R"(
// doptimize OSRs.
if (pass == optDuration) whenPass = () => {};
whenPass(pass, optDuration);
while (Date.now() - startTime < pass) {
for (let i = 0; i < 1e5; i++) {
for (let j = 0; j < 1000; j++) {
x = Math.random() * j;
}
if ((Date.now() - startTime) > pass) break;
}
}
}
......@@ -2240,7 +2241,10 @@ static const char* pre_profiling_osr_script = R"(
)";
// Testing profiling of OSR code that was OSR optimized before profiling
// started.
// started. Currently the behavior is not quite right so we're currently
// testing a deopt event being sent to the sampling thread for a function
// it knows nothing about. This deopt does mean we start getting samples
// for hot so we expect some samples, just fewer than for notHot.
//
// We should get something like:
// 0 (root):0 3 0 #1
......@@ -2249,17 +2253,25 @@ static const char* pre_profiling_osr_script = R"(
// 85 hot:5 0 4 #6
// 0 whenPass:2 0 4 #3
// 0 startProfiling:0 2 0 #4
//
// But currently get something like:
// 0 (root):0 3 0 #1
// 12 (garbage collector):0 3 0 #5
// 57 notHot:22 0 4 #2
// 33 hot:5 0 4 #6
// 0 whenPass:2 0 4 #3
// 0 startProfiling:0 2 0 #4
TEST(StartProfilingAfterOsr) {
i::FLAG_allow_natives_syntax = true;
v8::HandleScope scope(CcTest::isolate());
v8::Local<v8::Context> env = CcTest::NewContext({PROFILER_EXTENSION_ID});
v8::Context::Scope context_scope(env);
ProfilerHelper helper(env);
helper.profiler()->SetSamplingInterval(100);
CompileRun(pre_profiling_osr_script);
v8::Local<v8::Function> function = GetFunction(env, "notHot");
int32_t profiling_optimized_ms = 120;
int32_t profiling_optimized_ms = 80;
int32_t profiling_deoptimized_ms = 40;
v8::Local<v8::Value> args[] = {
v8::Integer::New(env->GetIsolate(), profiling_optimized_ms),
......@@ -2272,8 +2284,10 @@ TEST(StartProfilingAfterOsr) {
const CpuProfileNode* root = profile->GetTopDownRoot();
const v8::CpuProfileNode* notHotNode = GetChild(env, root, "notHot");
const v8::CpuProfileNode* hotNode = GetChild(env, notHotNode, "hot");
CHECK_GT(hotNode->GetHitCount(), notHotNode->GetHitCount());
USE(hotNode);
// If/when OSR sampling is fixed the following CHECK_GT could/should be
// uncommented and the node = node line deleted.
// CHECK_GT(hotNode->GetHitCount(), notHotNode->GetHitCount());
}
TEST(DontStopOnFinishedProfileDelete) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment