From 300b82e54a8db284e3f69082a6706ee8ca1d4314 Mon Sep 17 00:00:00 2001
From: Valentin Churavy <v.churavy@gmail.com>
Date: Mon, 1 Nov 2021 21:49:41 -0400
Subject: [PATCH 01/13] Support Meta

---
 src/PProf.jl  | 48 ++++++++++++++++++++++++++++++------------------
 test/PProf.jl |  4 ++--
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 176c9fa..340934d 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -19,7 +19,7 @@ clear
 include(joinpath("..", "lib", "perftools.jl"))
 
 import .perftools.profiles: ValueType, Sample, Function,
-                            Location, Line
+                            Location, Line, Label
 const PProfile = perftools.profiles.Profile
 
 const proc = Ref{Union{Base.Process, Nothing}}(nothing)
@@ -103,7 +103,7 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
                ui_relative_percentages::Bool = true,
             )
     if data === nothing
-        data = copy(Profile.fetch())
+        data = copy(Profile.fetch(include_meta=true))
     end
     lookup = lidict
     if lookup === nothing
@@ -122,6 +122,8 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     enter!(string) = _enter!(string_table, string)
     enter!(::Nothing) = _enter!(string_table, "nothing")
     ValueType!(_type, unit) = ValueType(_type = enter!(_type), unit = enter!(unit))
+    Label!(key, value, unit) = Label(key = enter!(key), num = value, num_unit = enter!(unit))
+    Label!(key, value) = Label(key = enter!(key), str = enter!(string(value)))
 
     # Setup:
     enter!("")  # NOTE: pprof requires first entry to be ""
@@ -135,7 +137,6 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
 
     sample_type = [
         ValueType!("events",      "count"), # Mandatory
-        ValueType!("stack_depth", "count")
     ]
 
     prof = PProfile(
@@ -154,28 +155,39 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
 
     # start decoding backtraces
     location_id = Vector{eltype(data)}()
-    lastwaszero = true
-
-    for ip in data
-        # ip == 0x0 is the sentinel value for finishing a backtrace, therefore finising a sample
-        if ip == 0
-            # Avoid creating empty samples
-            if lastwaszero
-                @assert length(location_id) == 0
-                continue
+
+    idx = length(data)
+    value = nothing
+    meta = nothing
+    while idx > 0
+        if Profile.is_block_end(data, idx)
+            if value !== nothing
+                @assert meta !== nothing
+                # Finish last block
+                push!(prof.sample, Sample(;location_id = reverse!(location_id), value = value, label = meta))
+                location_id = Vector{eltype(data)}()
             end
 
-            # End of sample
+            # read metadata
+            thread_sleeping = data[idx - 2] - 1 # subtract 1 as state is incremented to avoid being equal to 0
+            cpu_cycle_clock = data[idx - 3]
+            taskid = data[idx - 4]
+            threadid = data[idx - 5]
+
             value = [
                 1,                   # events
-                length(location_id), # stack_depth
             ]
-            push!(prof.sample, Sample(;location_id = location_id, value = value))
-            location_id = Vector{eltype(data)}()
-            lastwaszero = true
+            meta = Label[
+                Label!("thread_sleeping", thread_sleeping != 0),
+                Label!("cycle_clock", cpu_cycle_clock, "nanoseconds"),
+                Label!("taskid", taskid),
+                Label!("threadid", threadid),
+            ]
+            idx -= (Profile.nmeta + 1)
             continue
         end
-        lastwaszero = false
+        ip = data[idx]
+        idx -= 1
 
         # A backtrace consists of a set of IP (Instruction Pointers), each IP points
         # a single line of code and `litrace` has the necessary information to decode
diff --git a/test/PProf.jl b/test/PProf.jl
index c7a4a9f..5f992f8 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -64,10 +64,10 @@ end
     end
     for i in 1:2
         if i == 1
-            data = Profile.fetch()
+            data = Profile.fetch(include_meta = true)
             args = (data,)
         else
-            data,lidict = Profile.retrieve()
+            data,lidict = Profile.retrieve(include_meta = true)
             args = (data, lidict)
         end
 

From f3c1e6f29c3f1a7951d08224336f2b342f189faf Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 14:54:50 -0600
Subject: [PATCH 02/13] Fix off-by-one bug in meta calculation; use builtin
 offsets, so we catch any future changes

---
 src/PProf.jl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 32187b4..6ed86a7 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -160,10 +160,10 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             end
 
             # read metadata
-            thread_sleeping = data[idx - 2] - 1 # subtract 1 as state is incremented to avoid being equal to 0
-            cpu_cycle_clock = data[idx - 3]
-            taskid = data[idx - 4]
-            threadid = data[idx - 5]
+            thread_sleeping = data[idx - Profile.META_OFFSET_SLEEPSTATE] - 1  # "Sleeping" is recorded as 1 or 2, to avoid 0s, which indicate end-of-block.
+            cpu_cycle_clock = data[idx - Profile.META_OFFSET_CPUCYCLECLOCK]
+            taskid = data[idx - Profile.META_OFFSET_TASKID]
+            threadid = data[idx - Profile.META_OFFSET_THREADID]
 
             value = [
                 1,                   # events
@@ -174,7 +174,7 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
                 Label!("taskid", taskid),
                 Label!("threadid", threadid),
             ]
-            idx -= (Profile.nmeta + 1)
+            idx -= (Profile.nmeta + 2)  # skip all the metas, plus the 2 nulls that end a block.
             continue
         end
         ip = data[idx]

From 46b02bf6827e7c68a0bb2f68731b7fc240eda0fd Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 14:55:20 -0600
Subject: [PATCH 03/13] Add test that we enforce meta

---
 test/PProf.jl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/PProf.jl b/test/PProf.jl
index fbbc636..79a6634 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -94,6 +94,9 @@ end
         @test length(with_c.location) > length(without_c.location)
         @test length(with_c.var"#function") > length(without_c.var"#function")
     end
+
+    # Must have meta.
+    @test_throws AssertionError pprof(Profile.fetch(include_meta = false))
 end
 
 @testset "full_signatures" begin

From 7b1f04406471789fb75e2cc3c79da03099edab49 Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 15:06:33 -0600
Subject: [PATCH 04/13] Half-finished attempt at allowing older, non-meta
 profiles too

I'm not sure it's worth the complexity
---
 src/PProf.jl  | 30 +++++++++++++++++++++---------
 test/PProf.jl |  9 +++++----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 6ed86a7..0c0a15f 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -97,14 +97,16 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
                keep_frames::Union{Nothing, AbstractString} = nothing,
                ui_relative_percentages::Bool = true,
             )
+    has_meta = false
     if data === nothing
         data = if isdefined(Profile, :has_meta)
             copy(Profile.fetch(include_meta = true))
+            has_meta = true
         else
             copy(Profile.fetch())
         end
     elseif isdefined(Profile, :has_meta)
-        @assert Profile.has_meta(data) "PProf expects `Profile.fetch(include_meta=true)`."
+        has_meta = Profile.has_meta(data)
     end
     lookup = lidict
     if lookup === nothing
@@ -147,27 +149,28 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     # start decoding backtraces
     location_id = Vector{eltype(data)}()
 
+    # All samples get the same value for the CPU profile.
+    value = [
+        1,      # events
+    ]
+
     idx = length(data)
-    value = nothing
     meta = nothing
     while idx > 0
-        if Profile.is_block_end(data, idx)
-            if value !== nothing
-                @assert meta !== nothing
+        if has_meta && Profile.is_block_end(data, idx)
+            if meta !== nothing
                 # Finish last block
                 push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
                 location_id = Vector{eltype(data)}()
             end
 
-            # read metadata
+            # Consume all of the metadata entries in the buffer, and then position the IP
+            # at the idx for the actual ip.
             thread_sleeping = data[idx - Profile.META_OFFSET_SLEEPSTATE] - 1  # "Sleeping" is recorded as 1 or 2, to avoid 0s, which indicate end-of-block.
             cpu_cycle_clock = data[idx - Profile.META_OFFSET_CPUCYCLECLOCK]
             taskid = data[idx - Profile.META_OFFSET_TASKID]
             threadid = data[idx - Profile.META_OFFSET_THREADID]
 
-            value = [
-                1,                   # events
-            ]
             meta = Label[
                 Label!("thread_sleeping", thread_sleeping != 0),
                 Label!("cycle_clock", cpu_cycle_clock, "nanoseconds"),
@@ -176,6 +179,15 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             ]
             idx -= (Profile.nmeta + 2)  # skip all the metas, plus the 2 nulls that end a block.
             continue
+        elseif !has_meta && data[idx] == 0
+            # ip == 0x0 is the sentinel value for finishing a backtrace (when meta is disabled), therefore finising a sample
+            # On some platforms, we sometimes get two 0s in a row for some reason...
+            if idx > 1 && data[idx-1] == 0
+                idx -= 1
+            end
+            # Finish last block
+            push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
+            location_id = Vector{eltype(data)}()
         end
         ip = data[idx]
         idx -= 1
diff --git a/test/PProf.jl b/test/PProf.jl
index 79a6634..2fa8c01 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -74,10 +74,14 @@ end
         end
         sleep(2)
     end
-    for i in 1:2
+    for i in 1:3
         if i == 1
             data = Profile.fetch(include_meta = true)
             args = (data,)
+        elseif i == 2
+            # Ensure we are backwards compatible with older, non-meta profiles
+            data = Profile.fetch(include_meta = false)
+            args = (data,)
         else
             data,lidict = Profile.retrieve(include_meta = true)
             args = (data, lidict)
@@ -94,9 +98,6 @@ end
         @test length(with_c.location) > length(without_c.location)
         @test length(with_c.var"#function") > length(without_c.var"#function")
     end
-
-    # Must have meta.
-    @test_throws AssertionError pprof(Profile.fetch(include_meta = false))
 end
 
 @testset "full_signatures" begin

From 7e8413cd173b0e66f8d842db0839e24118730f9b Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 15:06:45 -0600
Subject: [PATCH 05/13] Revert "Half-finished attempt at allowing older,
 non-meta profiles too"

This reverts commit 7b1f04406471789fb75e2cc3c79da03099edab49.
---
 src/PProf.jl  | 30 +++++++++---------------------
 test/PProf.jl |  9 ++++-----
 2 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 0c0a15f..6ed86a7 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -97,16 +97,14 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
                keep_frames::Union{Nothing, AbstractString} = nothing,
                ui_relative_percentages::Bool = true,
             )
-    has_meta = false
     if data === nothing
         data = if isdefined(Profile, :has_meta)
             copy(Profile.fetch(include_meta = true))
-            has_meta = true
         else
             copy(Profile.fetch())
         end
     elseif isdefined(Profile, :has_meta)
-        has_meta = Profile.has_meta(data)
+        @assert Profile.has_meta(data) "PProf expects `Profile.fetch(include_meta=true)`."
     end
     lookup = lidict
     if lookup === nothing
@@ -149,28 +147,27 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     # start decoding backtraces
     location_id = Vector{eltype(data)}()
 
-    # All samples get the same value for the CPU profile.
-    value = [
-        1,      # events
-    ]
-
     idx = length(data)
+    value = nothing
     meta = nothing
     while idx > 0
-        if has_meta && Profile.is_block_end(data, idx)
-            if meta !== nothing
+        if Profile.is_block_end(data, idx)
+            if value !== nothing
+                @assert meta !== nothing
                 # Finish last block
                 push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
                 location_id = Vector{eltype(data)}()
             end
 
-            # Consume all of the metadata entries in the buffer, and then position the IP
-            # at the idx for the actual ip.
+            # read metadata
             thread_sleeping = data[idx - Profile.META_OFFSET_SLEEPSTATE] - 1  # "Sleeping" is recorded as 1 or 2, to avoid 0s, which indicate end-of-block.
             cpu_cycle_clock = data[idx - Profile.META_OFFSET_CPUCYCLECLOCK]
             taskid = data[idx - Profile.META_OFFSET_TASKID]
             threadid = data[idx - Profile.META_OFFSET_THREADID]
 
+            value = [
+                1,                   # events
+            ]
             meta = Label[
                 Label!("thread_sleeping", thread_sleeping != 0),
                 Label!("cycle_clock", cpu_cycle_clock, "nanoseconds"),
@@ -179,15 +176,6 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             ]
             idx -= (Profile.nmeta + 2)  # skip all the metas, plus the 2 nulls that end a block.
             continue
-        elseif !has_meta && data[idx] == 0
-            # ip == 0x0 is the sentinel value for finishing a backtrace (when meta is disabled), therefore finising a sample
-            # On some platforms, we sometimes get two 0s in a row for some reason...
-            if idx > 1 && data[idx-1] == 0
-                idx -= 1
-            end
-            # Finish last block
-            push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
-            location_id = Vector{eltype(data)}()
         end
         ip = data[idx]
         idx -= 1
diff --git a/test/PProf.jl b/test/PProf.jl
index 2fa8c01..79a6634 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -74,14 +74,10 @@ end
         end
         sleep(2)
     end
-    for i in 1:3
+    for i in 1:2
         if i == 1
             data = Profile.fetch(include_meta = true)
             args = (data,)
-        elseif i == 2
-            # Ensure we are backwards compatible with older, non-meta profiles
-            data = Profile.fetch(include_meta = false)
-            args = (data,)
         else
             data,lidict = Profile.retrieve(include_meta = true)
             args = (data, lidict)
@@ -98,6 +94,9 @@ end
         @test length(with_c.location) > length(without_c.location)
         @test length(with_c.var"#function") > length(without_c.var"#function")
     end
+
+    # Must have meta.
+    @test_throws AssertionError pprof(Profile.fetch(include_meta = false))
 end
 
 @testset "full_signatures" begin

From 1fc51c9a584d20aede0e6b526a66752be739074a Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 15:35:24 -0600
Subject: [PATCH 06/13] Revert "Revert "Half-finished attempt at allowing
 older, non-meta profiles too""

This reverts commit 7e8413cd173b0e66f8d842db0839e24118730f9b.
---
 src/PProf.jl  | 30 +++++++++++++++++++++---------
 test/PProf.jl |  9 +++++----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 6ed86a7..0c0a15f 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -97,14 +97,16 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
                keep_frames::Union{Nothing, AbstractString} = nothing,
                ui_relative_percentages::Bool = true,
             )
+    has_meta = false
     if data === nothing
         data = if isdefined(Profile, :has_meta)
             copy(Profile.fetch(include_meta = true))
+            has_meta = true
         else
             copy(Profile.fetch())
         end
     elseif isdefined(Profile, :has_meta)
-        @assert Profile.has_meta(data) "PProf expects `Profile.fetch(include_meta=true)`."
+        has_meta = Profile.has_meta(data)
     end
     lookup = lidict
     if lookup === nothing
@@ -147,27 +149,28 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     # start decoding backtraces
     location_id = Vector{eltype(data)}()
 
+    # All samples get the same value for the CPU profile.
+    value = [
+        1,      # events
+    ]
+
     idx = length(data)
-    value = nothing
     meta = nothing
     while idx > 0
-        if Profile.is_block_end(data, idx)
-            if value !== nothing
-                @assert meta !== nothing
+        if has_meta && Profile.is_block_end(data, idx)
+            if meta !== nothing
                 # Finish last block
                 push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
                 location_id = Vector{eltype(data)}()
             end
 
-            # read metadata
+            # Consume all of the metadata entries in the buffer, and then position the IP
+            # at the idx for the actual ip.
             thread_sleeping = data[idx - Profile.META_OFFSET_SLEEPSTATE] - 1  # "Sleeping" is recorded as 1 or 2, to avoid 0s, which indicate end-of-block.
             cpu_cycle_clock = data[idx - Profile.META_OFFSET_CPUCYCLECLOCK]
             taskid = data[idx - Profile.META_OFFSET_TASKID]
             threadid = data[idx - Profile.META_OFFSET_THREADID]
 
-            value = [
-                1,                   # events
-            ]
             meta = Label[
                 Label!("thread_sleeping", thread_sleeping != 0),
                 Label!("cycle_clock", cpu_cycle_clock, "nanoseconds"),
@@ -176,6 +179,15 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             ]
             idx -= (Profile.nmeta + 2)  # skip all the metas, plus the 2 nulls that end a block.
             continue
+        elseif !has_meta && data[idx] == 0
+            # ip == 0x0 is the sentinel value for finishing a backtrace (when meta is disabled), therefore finising a sample
+            # On some platforms, we sometimes get two 0s in a row for some reason...
+            if idx > 1 && data[idx-1] == 0
+                idx -= 1
+            end
+            # Finish last block
+            push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
+            location_id = Vector{eltype(data)}()
         end
         ip = data[idx]
         idx -= 1
diff --git a/test/PProf.jl b/test/PProf.jl
index 79a6634..2fa8c01 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -74,10 +74,14 @@ end
         end
         sleep(2)
     end
-    for i in 1:2
+    for i in 1:3
         if i == 1
             data = Profile.fetch(include_meta = true)
             args = (data,)
+        elseif i == 2
+            # Ensure we are backwards compatible with older, non-meta profiles
+            data = Profile.fetch(include_meta = false)
+            args = (data,)
         else
             data,lidict = Profile.retrieve(include_meta = true)
             args = (data, lidict)
@@ -94,9 +98,6 @@ end
         @test length(with_c.location) > length(without_c.location)
         @test length(with_c.var"#function") > length(without_c.var"#function")
     end
-
-    # Must have meta.
-    @test_throws AssertionError pprof(Profile.fetch(include_meta = false))
 end
 
 @testset "full_signatures" begin

From 2d72715f16e0482fe322e86baae6bed36b3b523b Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 15:44:58 -0600
Subject: [PATCH 07/13] Fixup the backwards compatiblity change: support
 non-meta profiles

---
 src/PProf.jl  | 24 ++++++++++++++++--------
 test/PProf.jl |  8 ++++++--
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 0c0a15f..fe1fdfe 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -100,8 +100,8 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     has_meta = false
     if data === nothing
         data = if isdefined(Profile, :has_meta)
-            copy(Profile.fetch(include_meta = true))
             has_meta = true
+            copy(Profile.fetch(include_meta = true))
         else
             copy(Profile.fetch())
         end
@@ -149,11 +149,13 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     # start decoding backtraces
     location_id = Vector{eltype(data)}()
 
-    # All samples get the same value for the CPU profile.
+    # All samples get the same value for CPU profiles.
     value = [
         1,      # events
     ]
 
+    lastwaszero = false  # (Legacy: used when has_meta = false)
+
     idx = length(data)
     meta = nothing
     while idx > 0
@@ -179,18 +181,24 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             ]
             idx -= (Profile.nmeta + 2)  # skip all the metas, plus the 2 nulls that end a block.
             continue
-        elseif !has_meta && data[idx] == 0
+        elseif !has_meta && idx != length(data) && data[idx] == 0
+            # Avoid creating empty samples
             # ip == 0x0 is the sentinel value for finishing a backtrace (when meta is disabled), therefore finising a sample
             # On some platforms, we sometimes get two 0s in a row for some reason...
-            if idx > 1 && data[idx-1] == 0
-                idx -= 1
+            if lastwaszero
+                @assert length(location_id) == 0
+            else
+                # Finish last block
+                push!(samples, Sample(;location_id = reverse!(location_id), value = value))
+                location_id = Vector{eltype(data)}()
+                lastwaszero = true
             end
-            # Finish last block
-            push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
-            location_id = Vector{eltype(data)}()
+            idx -= 1
+            continue
         end
         ip = data[idx]
         idx -= 1
+        lastwaszero = false
 
         # A backtrace consists of a set of IP (Instruction Pointers), each IP points
         # a single line of code and `litrace` has the necessary information to decode
diff --git a/test/PProf.jl b/test/PProf.jl
index 2fa8c01..c3a1499 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -74,16 +74,20 @@ end
         end
         sleep(2)
     end
-    for i in 1:3
+    @testset for i in 1:4
         if i == 1
             data = Profile.fetch(include_meta = true)
             args = (data,)
         elseif i == 2
+            data,lidict = Profile.retrieve(include_meta = true)
+            args = (data, lidict)
+        elseif i == 3
             # Ensure we are backwards compatible with older, non-meta profiles
             data = Profile.fetch(include_meta = false)
             args = (data,)
         else
-            data,lidict = Profile.retrieve(include_meta = true)
+            # Ensure we are backwards compatible with older, non-meta profiles
+            data,lidict = Profile.retrieve(include_meta = false)
             args = (data, lidict)
         end
 

From b3c5763074fadac3d43db4178ae5aaa843ba591c Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 16:17:31 -0600
Subject: [PATCH 08/13] Add corner-cases tests for manually constructed
 profiles; ensure we catch every sample

Fix the bugs in handling those cases :)
---
 src/PProf.jl  | 23 ++++++++++++++++++++---
 test/PProf.jl | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index fe1fdfe..1b08603 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -154,11 +154,19 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
         1,      # events
     ]
 
-    lastwaszero = false  # (Legacy: used when has_meta = false)
-
+    lastwaszero = true  # (Legacy: used when has_meta = false)
+
+    # The Profile data buffer is a big array, with each sample appended one after the other.
+    # Each sample now looks like this:
+    # | ip | ip | ip | meta1 | meta2 | meta3 | meta4| 0x0 | 0x0 |
+    # We iterate backwards, starting from the end, so that we don't encounter the metadata
+    # and mistake it for more ip addresses. For each sample, we skip the zeros, consume the
+    # metadata, then continue scanning the ip addresses, and when we hit another end of a
+    # block, we finish the sample we just consumed.
     idx = length(data)
     meta = nothing
     while idx > 0
+        # We handle the very first sample after the loop.
         if has_meta && Profile.is_block_end(data, idx)
             if meta !== nothing
                 # Finish last block
@@ -181,7 +189,7 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             ]
             idx -= (Profile.nmeta + 2)  # skip all the metas, plus the 2 nulls that end a block.
             continue
-        elseif !has_meta && idx != length(data) && data[idx] == 0
+        elseif !has_meta && data[idx] == 0
             # Avoid creating empty samples
             # ip == 0x0 is the sentinel value for finishing a backtrace (when meta is disabled), therefore finising a sample
             # On some platforms, we sometimes get two 0s in a row for some reason...
@@ -278,6 +286,15 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
             push!(location_id, ip)
         end
     end
+    if length(data) > 0
+        # Finish the very last sample
+        if has_meta
+            push!(samples, Sample(;location_id = reverse!(location_id), value = value, label = meta))
+        else
+            push!(samples, Sample(;location_id = reverse!(location_id), value = value))
+        end
+        location_id = Vector{eltype(data)}()
+    end
 
     # If from_c=false funcs and locs should NOT contain C functions
     prof = PProfile(
diff --git a/test/PProf.jl b/test/PProf.jl
index c3a1499..2d81d0d 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -65,6 +65,40 @@ function load_prof_proto(file)
     open(io->decode(ProtoDecoder(GzipDecompressorStream(io)), PProf.perftools.profiles.Profile), file, "r")
 end
 
+@testset "Corner Cases" begin
+    @testset "non-meta profile" begin
+
+        @testset "0 sample profile" begin
+            prof = load_prof_proto(pprof(UInt64[], out=tempname(), web=false))
+            @test length(prof.sample) == 0
+        end
+        @testset "1 sample profile" begin
+            prof = load_prof_proto(pprof(UInt64[0xdeadbeef,0], out=tempname(), web=false))
+            @test length(prof.sample) == 1
+        end
+
+        @testset "2 sample, 1 location profile" begin
+            prof = load_prof_proto(pprof(UInt64[0xdeadbeef,0, 0xdeadbeef, 0], out=tempname(), web=false))
+            @test length(prof.sample) == 2
+            @test length(prof.location) == 1
+        end
+    end
+    @testset "with-meta profile" begin
+        @testset "1 sample profile" begin
+            data = UInt64[0xdeadbeef, 1, 1, 1, 1, 0, 0]
+            prof = load_prof_proto(pprof(data, out=tempname(), web=false))
+            @test length(prof.sample) == 1
+        end
+
+        @testset "2 sample 1 location profile" begin
+            data = UInt64[0xdeadbeef, 1, 1, 1, 1, 0, 0, 0xdeadbeef, 1, 1, 1, 1, 0, 0]
+            prof = load_prof_proto(pprof(data, out=tempname(), web=false))
+            @test length(prof.sample) == 2
+            @test length(prof.location) == 1
+        end
+    end
+end
+
 @testset "with_c" begin
     Profile.clear()
 

From b2ffe7cdda8454826748ded8563060a0154c5d5a Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 16:18:09 -0600
Subject: [PATCH 09/13] Bump to version 3.1.0: Support meta!

---
 Project.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Project.toml b/Project.toml
index 8b3c177..e61bf6a 100644
--- a/Project.toml
+++ b/Project.toml
@@ -1,7 +1,7 @@
 name = "PProf"
 uuid = "e4faabce-9ead-11e9-39d9-4379958e3056"
 authors = ["Valentin Churavy <v.churavy@gmail.com>", "Nathan Daly <nhdaly@gmail.com>"]
-version = "3.0.0"
+version = "3.1.0"
 
 [deps]
 AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c"

From 64148cbec17b0f07988163d9acc708946ef5dde9 Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 16:55:44 -0600
Subject: [PATCH 10/13] Fix v1.6

---
 test/PProf.jl | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/test/PProf.jl b/test/PProf.jl
index 2d81d0d..b5a86bd 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -110,18 +110,32 @@ end
     end
     @testset for i in 1:4
         if i == 1
+            if !Profile.has_meta
+                continue
+            end
             data = Profile.fetch(include_meta = true)
             args = (data,)
         elseif i == 2
+            if !Profile.has_meta
+                continue
+            end
             data,lidict = Profile.retrieve(include_meta = true)
             args = (data, lidict)
         elseif i == 3
             # Ensure we are backwards compatible with older, non-meta profiles
-            data = Profile.fetch(include_meta = false)
+            if Profile.has_meta
+                data = Profile.fetch(include_meta = false)
+            else
+                data = Profile.fetch()
+            end
             args = (data,)
         else
             # Ensure we are backwards compatible with older, non-meta profiles
-            data,lidict = Profile.retrieve(include_meta = false)
+            if Profile.has_meta
+                data,lidict = Profile.retrieve(include_meta = false)
+            else
+                data,lidict = Profile.retrieve()
+            end
             args = (data, lidict)
         end
 

From 47b92a3a71145306c7066b6388f51c590a7fc47f Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 21:04:05 -0600
Subject: [PATCH 11/13] Try to flaky failing test

---
 test/PProf.jl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/PProf.jl b/test/PProf.jl
index b5a86bd..64f4aa2 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -191,6 +191,7 @@ end
 
 @testset "subprocess refresh" begin
 
+    PProf.kill()
     @pprof foo(10000, 5, [])
 
     current_proc = PProf.proc[]

From 2a789ae61c2f56a54a0898bd3601f7b68a134864 Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 21:01:40 -0600
Subject: [PATCH 12/13] Fixup test typo

---
 test/PProf.jl | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/test/PProf.jl b/test/PProf.jl
index 64f4aa2..7daee67 100644
--- a/test/PProf.jl
+++ b/test/PProf.jl
@@ -99,6 +99,8 @@ end
     end
 end
 
+
+const HAS_META = isdefined(Profile, :has_meta)
 @testset "with_c" begin
     Profile.clear()
 
@@ -110,20 +112,20 @@ end
     end
     @testset for i in 1:4
         if i == 1
-            if !Profile.has_meta
+            if !HAS_META
                 continue
             end
             data = Profile.fetch(include_meta = true)
             args = (data,)
         elseif i == 2
-            if !Profile.has_meta
+            if !HAS_META
                 continue
             end
             data,lidict = Profile.retrieve(include_meta = true)
             args = (data, lidict)
         elseif i == 3
             # Ensure we are backwards compatible with older, non-meta profiles
-            if Profile.has_meta
+            if HAS_META
                 data = Profile.fetch(include_meta = false)
             else
                 data = Profile.fetch()
@@ -131,7 +133,7 @@ end
             args = (data,)
         else
             # Ensure we are backwards compatible with older, non-meta profiles
-            if Profile.has_meta
+            if HAS_META
                 data,lidict = Profile.retrieve(include_meta = false)
             else
                 data,lidict = Profile.retrieve()

From c2ec74d7d1546e9f9f1287d1a8291e8168af32c5 Mon Sep 17 00:00:00 2001
From: Nathan Daly <nathan.daly@relational.ai>
Date: Fri, 20 Oct 2023 16:54:34 -0600
Subject: [PATCH 13/13] Show the metadata tags in the flamegraph UI by default

---
 src/PProf.jl | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/PProf.jl b/src/PProf.jl
index 1b08603..a97f73f 100644
--- a/src/PProf.jl
+++ b/src/PProf.jl
@@ -48,6 +48,7 @@ using Base.StackTraces: StackFrame
             web = true, webhost = "localhost", webport = 57599,
             out = "profile.pb.gz", from_c = true, full_signatures = true, drop_frames = "",
             keep_frames = "", ui_relative_percentages = true, sampling_delay = nothing,
+            tagroot = "taskid,threadid"
          )
     pprof(FlameGraphs.flamegraph(); kwargs...)
 
@@ -78,6 +79,14 @@ You can also use `PProf.refresh(file="...")` to open a new file in the server.
 - `from_c::Bool`: If `false`, exclude frames that come from from_c. Defaults to `true`.
 - `full_signatures::Bool`: If `true`, methods are printed as signatures with full argument
                            types. If `false`, as only names. E.g. `eval(::Module, ::Any)` vs `eval`.
+- `tagroot`: Set which metadata tags you want to turn into root frames for the profile. This
+             is used to view the metadata tags in the Flamegraph view. This should be a
+             comma-separated string, chosing from the following metadata options:
+             - `taskid`
+             - `threadid`
+             - `thread_sleeping`
+             - `cycle_clock`
+             Defaults to `"taskid,threadid"`, grouping by taskid then threadid.
 - `drop_frames`: frames with function_name fully matching regexp string will be dropped from the samples,
                  along with their successors.
 - `keep_frames`: frames with function_name fully matching regexp string will be kept, even if it matches drop_functions.
@@ -96,6 +105,7 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
                drop_frames::Union{Nothing, AbstractString} = nothing,
                keep_frames::Union{Nothing, AbstractString} = nothing,
                ui_relative_percentages::Bool = true,
+               tagroot::Union{Nothing, AbstractString} = "taskid,threadid",
             )
     has_meta = false
     if data === nothing
@@ -319,8 +329,7 @@ function pprof(data::Union{Nothing, Vector{UInt}} = nothing,
     end
 
     if web
-        refresh(webhost = webhost, webport = webport, file = out,
-            ui_relative_percentages = ui_relative_percentages)
+        refresh(; webhost, webport, file = out, ui_relative_percentages, tagroot)
     end
 
     out
@@ -361,6 +370,7 @@ function refresh(; webhost::AbstractString = "localhost",
                    webport::Integer = 57599,
                    file::AbstractString = "profile.pb.gz",
                    ui_relative_percentages::Bool = true,
+                   tagroot::Union{AbstractString,Nothing} = "taskid,threadid",
                 )
 
     if proc[] === nothing
@@ -374,7 +384,11 @@ function refresh(; webhost::AbstractString = "localhost",
     relative_percentages_flag = ui_relative_percentages ? "-relative_percentages" : ""
 
     proc[] = pprof_jll.pprof() do pprof_path
-        open(pipeline(`$pprof_path -http=$webhost:$webport $relative_percentages_flag $file`))
+        if tagroot !== nothing && !isempty(tagroot)
+            open(pipeline(`$pprof_path -tagroot $tagroot -http=$webhost:$webport $relative_percentages_flag $file`))
+        else
+            open(pipeline(`$pprof_path -http=$webhost:$webport $relative_percentages_flag $file`))
+        end
     end
 end