Never ignore JitFramed flag #105850
Never ignore JitFramed flag #105850
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Do you understand why this produces better stacktraces? |
Benchmark results on Intel
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Intel
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Amd
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Amd
Flame graphs: Main vs PR 🔥 For clean |
It just seemed odd to me that JIT just ignores
it reproduces for both Intel and Amd and for both runs, so basically 4 exactly the same "diffs" for this part. Related: https://www.brendangregg.com/blog/2024-03-17/the-return-of-the-frame-pointers.html |
| // The VM sets JitFlags::JIT_FLAG_FRAMED for two reasons: (1) the DOTNET_JitFramed variable is set, or | ||
| // (2) the function is marked "noinline". The reason for #2 is that people mark functions | ||
| // noinline to ensure the show up on in a stack walk. But for AMD64, we don't need a frame |
There was a problem hiding this comment.
The deleted comment is still true on Windows. I guess it was written before we cared about non-Windows.
It makes sense on Windows x64. The RBP-frames are useless on Windows x64. I would like to understand why this helps. Our strategy for omitting the RBP-frames should not mess with RBP-based stackwalking on Windows x86 or Linux x64: We do not use EBP/RBP as a general purpose register, so the effect of methods without the frame should be similar to inlining or tailcalling. It should not break the stackwalking. |
This comment was marked as resolved.
This comment was marked as resolved.
Benchmark results on Intel
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Amd
Flame graphs: Main vs PR 🔥 For clean |
This comment was marked as resolved.
This comment was marked as resolved.
Benchmark results on Intel
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Amd
Flame graphs: Main vs PR 🔥 For clean |
This comment was marked as resolved.
This comment was marked as resolved.
1 similar comment
This comment was marked as resolved.
This comment was marked as resolved.
Benchmark results on Arm64
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Arm64
Flame graphs: Main vs PR 🔥 For clean |
|
I wonder if it's possible to add Tier name to symbols.. |
|
Test run with @EgorBot -arm64 -profiler using System;
using System.Text.Json;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
BenchmarkRunner.Run<MyBench>(args: args);
public class MyBench
{
static MyObj[] Data = TestData();
static TimeSpan TS = TimeSpan.FromDays(42);
public static MyObj[] TestData()
{
MyObj[] testData = new MyObj[100];
for (int i = 0; i < testData.Length; i++)
{
MyObj obj1 = new("Some long ASCII text bla bla bla", 42, Guid.NewGuid(), 3.14, TS, null);
MyObj obj2 = new("'')((*&&^%$@#$%$^&*())''';(*&^%$E##^%$&%^*(", i, Guid.NewGuid(), 3.14, TS, obj1);
testData[i] = obj2;
}
return testData;
}
[Benchmark]
public object JsonStatham() => JsonSerializer.Serialize(Data);
}
public record MyObj(string Name, int Age, Guid Id, double SomeFloat, TimeSpan Ts, MyObj? InnerObj); |
Benchmark results on Arm64
Flame graphs: Main vs PR 🔥 For clean |
|
@EgorBot -amd -intel -profiler using System;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
public class Bench
{
[Benchmark]
public void WB()
{
Foo foo = new Foo();
for (long i = 0; i < 200000000; i++)
foo.x = foo;
}
}
internal class Foo
{
public volatile Foo x;
} |
Benchmark results on Intel
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Amd
Flame graphs: Main vs PR 🔥 For clean |
|
@EgorBot -arm64 -profiler using System;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
public class Bench
{
[Benchmark]
public void WB()
{
Foo foo = new Foo();
for (long i = 0; i < 200000000; i++)
foo.x = foo;
}
}
internal class Foo
{
public volatile Foo x;
} |
Benchmark results on Arm64
Flame graphs: Main vs PR 🔥 For clean |
|
@EgorBot -intel -amd -profiler using System;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
public class Bench
{
[Benchmark]
public void WB()
{
Foo foo = new Foo();
for (long i = 0; i < 200000000; i++)
foo.x = foo;
}
}
internal class Foo
{
public volatile Foo x;
} |
Benchmark results on Intel
Flame graphs: Main vs PR 🔥 For clean |
Benchmark results on Amd
Flame graphs: Main vs PR 🔥 For clean |
Contributes to #105690