Skip to content

Commit be7f2ef

Browse files
committed
Address several review comments
1 parent 4b7579c commit be7f2ef

30 files changed

+435
-1124
lines changed

go.mod

-4
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ require (
2020
github.com/felixge/fgprof v0.9.4-0.20221116204635-ececf7638e93
2121
github.com/felixge/httpsnoop v1.0.4
2222
github.com/fsnotify/fsnotify v1.8.0
23-
github.com/go-delve/delve v1.23.1
24-
github.com/fsnotify/fsnotify v1.7.0
2523
github.com/go-kit/log v0.2.1
2624
github.com/gogo/protobuf v1.3.2
2725
github.com/gogo/status v1.1.1
@@ -40,8 +38,6 @@ require (
4038
github.com/grafana/pyroscope/api v0.4.0
4139
github.com/grafana/pyroscope/lidia v0.0.0-20250416154336-a5c33510d5ff
4240
github.com/grafana/regexp v0.0.0-20240518133315-a468a5bfb3bc
43-
github.com/grafana/pyroscope/lidia v0.0.0-20250416154336-a5c33510d5ff
44-
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db
4541
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
4642
github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1
4743
github.com/hashicorp/go-multierror v1.1.1

go.sum

-6
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,6 @@ github.com/fullstorydev/emulators/storage v0.0.0-20240401123056-edc69752f474 h1:
226226
github.com/fullstorydev/emulators/storage v0.0.0-20240401123056-edc69752f474/go.mod h1:PQwxF4UU8wuL+srGxr3BOhIW5zXqgucwVlO/nPZLsxw=
227227
github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E=
228228
github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ=
229-
github.com/go-delve/delve v1.23.1 h1:MtZ13ppptttkqSuvVnwJ5CPhIAzDiOwRrYuCk3ES7fU=
230-
github.com/go-delve/delve v1.23.1/go.mod h1:S3SLuEE2mn7wipKilTvk1p9HdTMnXXElcEpiZ+VcuqU=
231229
github.com/go-fonts/dejavu v0.3.4 h1:Qqyx9IOs5CQFxyWTdvddeWzrX0VNwUAvbmAzL0fpjbc=
232230
github.com/go-fonts/dejavu v0.3.4/go.mod h1:D1z0DglIz+lmpeNYMYlxW4r22IhcdOYnt+R3PShU/Kg=
233231
github.com/go-fonts/latin-modern v0.3.3 h1:g2xNgI8yzdNzIVm+qvbMryB6yGPe0pSMss8QT3QwlJ0=
@@ -385,10 +383,6 @@ github.com/grafana/pyroscope/lidia v0.0.0-20250416154336-a5c33510d5ff h1:9dI8HuA
385383
github.com/grafana/pyroscope/lidia v0.0.0-20250416154336-a5c33510d5ff/go.mod h1:3f/0/CzhTk1moGlVOp2nJQKCAL+4M5GTnkfDd5hqDIk=
386384
github.com/grafana/regexp v0.0.0-20240518133315-a468a5bfb3bc h1:GN2Lv3MGO7AS6PrRoT6yV5+wkrOpcszoIsO4+4ds248=
387385
github.com/grafana/regexp v0.0.0-20240518133315-a468a5bfb3bc/go.mod h1:+JKpmjMGhpgPL+rXZ5nsZieVzvarn86asRlBg4uNGnk=
388-
github.com/grafana/pyroscope/lidia v0.0.0-20250416154336-a5c33510d5ff h1:9dI8HuAIA5L/EsPXkk3YqA3Pp5i7ECvQkWMwUepLoCY=
389-
github.com/grafana/pyroscope/lidia v0.0.0-20250416154336-a5c33510d5ff/go.mod h1:3f/0/CzhTk1moGlVOp2nJQKCAL+4M5GTnkfDd5hqDIk=
390-
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db h1:7aN5cccjIqCLTzedH7MZzRZt5/lsAHch6Z3L2ZGn5FA=
391-
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db/go.mod h1:M5qHK+eWfAv8VR/265dIuEpL3fNfeC21tXXp9itM24A=
392386
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI=
393387
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0/go.mod h1:g5qyo/la0ALbONm6Vbp88Yd8NsDy6rZz+RcrMPxvld8=
394388
github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1 h1:VNqngBF40hVlDloBruUehVYC3ArSgIyScOAyMRqBxRg=

lidia/builder.go

+3-45
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package lidia
22

33
import (
44
"encoding/binary"
5-
"fmt"
65
"sort"
76
)
87

@@ -105,10 +104,9 @@ func (s *sortByVADepth) Swap(i, j int) {
105104

106105
// rangeCollector
107106
type rangeCollector struct {
108-
sb *stringBuilder
109-
rb *rangesBuilder
110-
lb *lineBuilder
111-
blb *binaryLayoutBuilder
107+
sb *stringBuilder
108+
rb *rangesBuilder
109+
lb *lineBuilder
112110

113111
opt options
114112
}
@@ -137,43 +135,3 @@ func (rc *rangeCollector) VisitRange(r *Range) {
137135
}
138136
rc.rb.add(r.VA, e)
139137
}
140-
141-
type binaryLayoutBuilder struct {
142-
buf []byte
143-
}
144-
145-
func newBinaryLayoutBuilder() *binaryLayoutBuilder {
146-
return &binaryLayoutBuilder{
147-
buf: make([]byte, 0, 256),
148-
}
149-
}
150-
151-
func (blb *binaryLayoutBuilder) write(layout *BinaryLayoutInfo) error {
152-
if layout == nil {
153-
return fmt.Errorf("nil binary layout")
154-
}
155-
156-
// Reset buffer
157-
blb.buf = blb.buf[:0]
158-
159-
// Write ELF type
160-
blb.buf = binary.LittleEndian.AppendUint16(blb.buf, layout.Type)
161-
162-
// Write number of program headers
163-
count := uint32(len(layout.ProgramHeaders))
164-
blb.buf = binary.LittleEndian.AppendUint32(blb.buf, count)
165-
166-
// Write each program header
167-
for _, ph := range layout.ProgramHeaders {
168-
blb.buf = binary.LittleEndian.AppendUint32(blb.buf, ph.Type)
169-
blb.buf = binary.LittleEndian.AppendUint32(blb.buf, ph.Flags)
170-
blb.buf = binary.LittleEndian.AppendUint64(blb.buf, ph.Offset)
171-
blb.buf = binary.LittleEndian.AppendUint64(blb.buf, ph.VirtualAddr)
172-
blb.buf = binary.LittleEndian.AppendUint64(blb.buf, ph.PhysAddr)
173-
blb.buf = binary.LittleEndian.AppendUint64(blb.buf, ph.FileSize)
174-
blb.buf = binary.LittleEndian.AppendUint64(blb.buf, ph.MemSize)
175-
blb.buf = binary.LittleEndian.AppendUint64(blb.buf, ph.Align)
176-
}
177-
178-
return nil
179-
}

lidia/constants.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,11 @@ const (
1111
version uint32 = 1
1212

1313
// Size of the file header in bytes
14-
headerSize = 0x98
14+
headerSize = 0x80
1515

1616
// Number of fields in a line table entry
1717
lineTableFieldsCount = 2
1818

19-
// Size of each line table entry (2 uint16s or 2 uint32s)
20-
lineTableFieldsSize = 4
21-
2219
// Number of fields in a range entry
2320
fieldsCount = 8
2421

lidia/format.go

-34
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,6 @@ type lineTablesHeader struct {
5454
_ uint32
5555
}
5656

57-
type binaryLayoutHeader struct {
58-
size uint64
59-
offset uint64
60-
crc uint32
61-
_ uint32
62-
}
63-
6457
type header struct {
6558
// 0x0
6659
magic [4]byte
@@ -74,8 +67,6 @@ type header struct {
7467
// 0x60
7568
lineTablesHeader lineTablesHeader
7669
// 0x80
77-
binaryLayoutHeader binaryLayoutHeader
78-
// 0x98
7970
}
8071

8172
func readHeader(file io.Reader) (header, error) {
@@ -106,10 +97,6 @@ func readHeader(file io.Reader) (header, error) {
10697
hdr.lineTablesHeader.offset = binary.LittleEndian.Uint64(headerBuf[0x70:])
10798
hdr.lineTablesHeader.crc = binary.LittleEndian.Uint32(headerBuf[0x78:])
10899

109-
hdr.binaryLayoutHeader.size = binary.LittleEndian.Uint64(headerBuf[0x80:])
110-
hdr.binaryLayoutHeader.offset = binary.LittleEndian.Uint64(headerBuf[0x88:])
111-
hdr.binaryLayoutHeader.crc = binary.LittleEndian.Uint32(headerBuf[0x90:])
112-
113100
return hdr, nil
114101
}
115102

@@ -179,23 +166,6 @@ func (rc *rangeCollector) write(f io.WriteSeeker) error {
179166
return err
180167
}
181168

182-
// Flush buffer to ensure line tables are written
183-
if err := buf.Flush(); err != nil {
184-
return err
185-
}
186-
187-
hdr.binaryLayoutHeader.offset = hdr.lineTablesHeader.offset + uint64(len(rc.lb.entries)*lineTableFieldsSize)
188-
hdr.binaryLayoutHeader.size = uint64(len(rc.blb.buf))
189-
190-
if len(rc.blb.buf) > 0 {
191-
if _, err := f.Write(rc.blb.buf); err != nil {
192-
return err
193-
}
194-
crc := crc32.New(castagnoli)
195-
_, _ = crc.Write(rc.blb.buf)
196-
hdr.binaryLayoutHeader.crc = crc.Sum32()
197-
}
198-
199169
if _, err := f.Seek(0, io.SeekStart); err != nil {
200170
return err
201171
}
@@ -230,10 +200,6 @@ func writeHeader(output io.Writer, hdr *header) error {
230200
binary.LittleEndian.PutUint64(headerBuf[0x70:], hdr.lineTablesHeader.offset)
231201
binary.LittleEndian.PutUint32(headerBuf[0x78:], hdr.lineTablesHeader.crc)
232202

233-
binary.LittleEndian.PutUint64(headerBuf[0x80:], hdr.binaryLayoutHeader.size)
234-
binary.LittleEndian.PutUint64(headerBuf[0x88:], hdr.binaryLayoutHeader.offset)
235-
binary.LittleEndian.PutUint32(headerBuf[0x90:], hdr.binaryLayoutHeader.crc)
236-
237203
if _, err := output.Write(headerBuf); err != nil {
238204
return err
239205
}

lidia/lidia.go

+1-103
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ package lidia
77

88
import (
99
"debug/elf"
10-
"encoding/binary"
1110
"fmt"
12-
"hash/crc32"
1311
"io"
1412
"os"
1513
"sort"
@@ -158,33 +156,7 @@ func CreateLidiaFromELF(elfFile *elf.File, output io.WriteSeeker, opts ...Option
158156
sb := newStringBuilder()
159157
rb := newRangesBuilder()
160158
lb := newLineTableBuilder()
161-
blb := newBinaryLayoutBuilder()
162-
rc := &rangeCollector{sb: sb, rb: rb, lb: lb, blb: blb}
163-
164-
// Extract binary layout
165-
layout := &BinaryLayoutInfo{
166-
Type: uint16(elfFile.Type),
167-
ProgramHeaders: make([]ProgramHeaderInfo, 0),
168-
}
169-
170-
for _, prog := range elfFile.Progs {
171-
if prog.Type == elf.PT_LOAD {
172-
layout.ProgramHeaders = append(layout.ProgramHeaders, ProgramHeaderInfo{
173-
Type: uint32(prog.Type),
174-
Flags: uint32(prog.Flags),
175-
Offset: prog.Off,
176-
VirtualAddr: prog.Vaddr,
177-
PhysAddr: prog.Paddr,
178-
FileSize: prog.Filesz,
179-
MemSize: prog.Memsz,
180-
Align: prog.Align,
181-
})
182-
}
183-
}
184-
185-
if err := blb.write(layout); err != nil {
186-
return fmt.Errorf("failed to write binary layout: %w", err)
187-
}
159+
rc := &rangeCollector{sb: sb, rb: rb, lb: lb}
188160

189161
for _, o := range opts {
190162
o(&rc.opt)
@@ -265,69 +237,6 @@ func (st *Table) Lookup(dst []SourceInfoFrame, addr uint64) ([]SourceInfoFrame,
265237
return dst, nil
266238
}
267239

268-
func (st *Table) GetBinaryLayout() (*BinaryLayoutInfo, error) {
269-
if st.hdr.binaryLayoutHeader.size == 0 {
270-
return nil, fmt.Errorf("binary layout information not available")
271-
}
272-
273-
// Read binary layout data
274-
data := make([]byte, st.hdr.binaryLayoutHeader.size)
275-
if _, err := st.file.ReadAt(data, int64(st.hdr.binaryLayoutHeader.offset)); err != nil {
276-
return nil, fmt.Errorf("failed to read binary layout: %w", err)
277-
}
278-
279-
// Verify CRC if enabled
280-
if st.opt.crc {
281-
crc := crc32.Checksum(data, crc32.MakeTable(crc32.Castagnoli))
282-
if crc != st.hdr.binaryLayoutHeader.crc {
283-
return nil, fmt.Errorf("binary layout CRC mismatch")
284-
}
285-
}
286-
287-
// Decode binary layout
288-
if len(data) < 6 {
289-
return nil, fmt.Errorf("binary layout data too short")
290-
}
291-
292-
layout := &BinaryLayoutInfo{
293-
Type: binary.LittleEndian.Uint16(data[:2]),
294-
}
295-
296-
count := binary.LittleEndian.Uint32(data[2:6])
297-
if count > 1000 {
298-
return nil, fmt.Errorf("invalid program header count: %d", count)
299-
}
300-
301-
expectedSize := uint64(6 + count*56)
302-
if uint64(len(data)) < expectedSize {
303-
return nil, fmt.Errorf("binary layout data truncated")
304-
}
305-
306-
layout.ProgramHeaders = make([]ProgramHeaderInfo, count)
307-
offset := 6
308-
for i := range layout.ProgramHeaders {
309-
ph := &layout.ProgramHeaders[i]
310-
ph.Type = binary.LittleEndian.Uint32(data[offset:])
311-
offset += 4
312-
ph.Flags = binary.LittleEndian.Uint32(data[offset:])
313-
offset += 4
314-
ph.Offset = binary.LittleEndian.Uint64(data[offset:])
315-
offset += 8
316-
ph.VirtualAddr = binary.LittleEndian.Uint64(data[offset:])
317-
offset += 8
318-
ph.PhysAddr = binary.LittleEndian.Uint64(data[offset:])
319-
offset += 8
320-
ph.FileSize = binary.LittleEndian.Uint64(data[offset:])
321-
offset += 8
322-
ph.MemSize = binary.LittleEndian.Uint64(data[offset:])
323-
offset += 8
324-
ph.Align = binary.LittleEndian.Uint64(data[offset:])
325-
offset += 8
326-
}
327-
328-
return layout, nil
329-
}
330-
331240
// Close releases resources associated with the Table.
332241
func (st *Table) Close() {
333242
if st.file != nil {
@@ -349,16 +258,5 @@ func (st *Table) CheckCRC() error {
349258
if err := st.CheckCRCLineTables(); err != nil {
350259
return err
351260
}
352-
353-
// Add binary layout CRC check
354-
if st.hdr.binaryLayoutHeader.size > 0 {
355-
if err := checkCRC(st.file,
356-
int64(st.hdr.binaryLayoutHeader.offset),
357-
int64(st.hdr.binaryLayoutHeader.size),
358-
st.hdr.binaryLayoutHeader.crc,
359-
"binary layout"); err != nil {
360-
return err
361-
}
362-
}
363261
return nil
364262
}

pkg/experiment/ingester/segment_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ func gprofileFromProtoProfile(t *testing.T, profile *profilev1.Profile) *gprofil
868868
func staticTestData() []inputChunk {
869869
return []inputChunk{
870870
{
871-
//todo check why it takes 10ms for each dataset
871+
//todo check why it takes 10ms for each head
872872
{shard: 1, tenant: "t1", profile: cpuProfile(42, 480, "svc1", "foo", "bar")},
873873
{shard: 1, tenant: "t1", profile: cpuProfile(13, 233, "svc1", "qwe", "foo", "bar")},
874874
{shard: 1, tenant: "t1", profile: cpuProfile(13, 472, "svc1", "qwe", "foo", "bar")},

0 commit comments

Comments
 (0)