On Wed, 9 Jun 2021 19:04:54 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> When the signal sent for AsyncGetCallTrace or JFR would land on a runtime
>> stub (like arraycopy), a vtable stub, or the prolog of a compiled method,
>> it wouldn't be able to detect the sender (caller) frame for multiple
>> reasons. This patch fixes these cases through adding CodeBlob-specific frame
>> parser which are in the best position to know how a frame is setup.
>>
>> The following examples have been profiled with honest-profiler which uses
>> `AsyncGetCallTrace`.
>>
>> # `Prof1`
>>
>> public class Prof1 {
>>
>> public static void main(String[] args) {
>> StringBuilder sb = new StringBuilder();
>> for (int i = 0; i < 1000000; i++) {
>> sb.append("ab");
>> sb.delete(0, 1);
>> }
>> System.out.println(sb.length());
>> }
>> }
>>
>>
>> - Baseline:
>>
>> Flat Profile (by method):
>> (t 99.4,s 99.4) AGCT::Unknown Java[ERR=-5]
>> (t 0.5,s 0.2) Prof1::main
>> (t 0.2,s 0.2) java.lang.AbstractStringBuilder::append
>> (t 0.1,s 0.1) AGCT::Unknown not Java[ERR=-3]
>> (t 0.0,s 0.0)
>> java.lang.AbstractStringBuilder::ensureCapacityInternal
>> (t 0.0,s 0.0) java.lang.AbstractStringBuilder::shift
>> (t 0.0,s 0.0) java.lang.String::getBytes
>> (t 0.0,s 0.0) java.lang.AbstractStringBuilder::putStringAt
>> (t 0.0,s 0.0) java.lang.StringBuilder::delete
>> (t 0.2,s 0.0) java.lang.StringBuilder::append
>> (t 0.0,s 0.0) java.lang.AbstractStringBuilder::delete
>> (t 0.0,s 0.0) java.lang.AbstractStringBuilder::putStringAt
>>
>> - With `StubRoutinesBlob::FrameParser`:
>>
>> Flat Profile (by method):
>> (t 98.7,s 98.7)
>> java.lang.AbstractStringBuilder::ensureCapacityInternal
>> (t 0.9,s 0.9) java.lang.AbstractStringBuilder::delete
>> (t 99.8,s 0.2) Prof1::main
>> (t 0.1,s 0.1) AGCT::Unknown not Java[ERR=-3]
>> (t 0.0,s 0.0) AGCT::Unknown Java[ERR=-5]
>> (t 98.8,s 0.0) java.lang.AbstractStringBuilder::append
>> (t 98.8,s 0.0) java.lang.StringBuilder::append
>> (t 0.9,s 0.0) java.lang.StringBuilder::delete
>>
>>
>> # `Prof2`
>>
>> import java.util.function.Supplier;
>>
>> public class Prof2 {
>>
>> public static void main(String[] args) {
>> var rand = new java.util.Random(0);
>> Supplier[] suppliers = {
>> () -> 0,
>> () -> 1,
>> () -> 2,
>> () -> 3,
>> };
>>
>> long sum = 0;
>> for (int i = 0; i >= 0; i++) {
>> sum += (int)suppliers[i % suppliers.length].get();
>> }
>> }
>> }
>>
>>
>> - Baseline:
>>
>> Flat Profile (by method):
>> (t 60.7,s 60.7) AGCT::Unknown Java[ERR=-5]
>> (t 39.2,s 35.2) Prof2::main
>> (t 1.4,s 1.4) Prof2::lambda$main$3
>> (t 1.0,s 1.0) Prof2::lambda$main$2
>> (t 0.9,s 0.9) Prof2::lambda$main$1
>> (t 0.7,s 0.7) Prof2::lambda$main$0
>> (t 0.1,s 0.1) AGCT::Unknown not Java[ERR=-3]
>> (t 0.0,s 0.0) java.lang.Thread::exit
>> (t 0.9,s 0.0) Prof2$$Lambda$2.0x0000000800c00c28::get
>> (t 1.0,s 0.0) Prof2$$Lambda$3.0x0000000800c01000::get
>> (t 1.4,s 0.0) Prof2$$Lambda$4.0x0000000800c01220::get
>> (t 0.7,s 0.0) Prof2$$Lambda$1.0x0000000800c00a08::get
>>
>>
>> - With `VtableBlob::FrameParser` and `nmethod::FrameParser`:
>>
>> Flat Profile (by method):
>> (t 74.1,s 70.3) Prof2::main
>> (t 6.5,s 5.5) Prof2$$Lambda$29.0x0000000800081220::get
>> (t 6.6,s 5.4) Prof2$$Lambda$28.0x0000000800081000::get
>> (t 5.7,s 5.0) Prof2$$Lambda$26.0x0000000800080a08::get
>> (t 5.9,s 5.0) Prof2$$Lambda$27.0x0000000800080c28::get
>> (t 4.9,s 4.9) AGCT::Unknown Java[ERR=-5]
>> (t 1.2,s 1.2) Prof2::lambda$main$2
>> (t 0.9,s 0.9) Prof2::lambda$main$3
>> (t 0.9,s 0.9) Prof2::lambda$main$1
>> (t 0.7,s 0.7) Prof2::lambda$main$0
>> (t 0.1,s 0.1) AGCT::Unknown not Java[ERR=-3]
>
> Hi Ludovic,
> Thank you for working on this fix in the AsyncGetCallTrace.
> What version of JDK release do you intent to target?
> Just wanted to make sure you know the JDK 17 development cycle will be closed
> tomorrow for P4 bugs and enhancements. The repository will be forked and the
> RDP 1 phase started.
> I doubt the review of your fix will be completed by this time.
> So, please, keep in mind your PR will go to 18, not 17.
> Thanks,
> Serguei
@sspitsyn thank you for the reminder. It's perfectly fine for this change to
land in JDK 18. We'll see in the future if there is a demand to backport it to
JDK 17 and we'll do accordingly.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4436
