On Thu, Sep 17, 2026 at 10:39:10AM +1000, Kugan Vivekanandarajah wrote:
> +/* Linux perf_event_attr field offsets. */
> +constexpr size_t PERF_EVENT_ATTR_SAMPLE_TYPE_OFF = 24;
> +constexpr size_t PERF_EVENT_ATTR_SIZE_FIELD_OFF = 4;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_FOR_SAMPLE_TYPE = 32;
> +constexpr size_t PERF_EVENT_ATTR_SAMPLE_PERIOD_OFF = 16;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_THROUGH_SAMPLE_PERIOD = 24;
> +constexpr size_t PERF_EVENT_ATTR_READ_FORMAT_OFF = 32;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_THROUGH_READ_FORMAT = 40;
> +constexpr size_t PERF_EVENT_ATTR_FLAGS_OFF = 40;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_THROUGH_FLAGS = 48;
> +constexpr uint64_t PERF_EVENT_ATTR_FREQ = 1ULL << 10;
> +constexpr size_t PERF_EVENT_ATTR_BRANCH_SAMPLE_TYPE_OFF = 72;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_THROUGH_BRANCH_SAMPLE_TYPE = 80;
> +constexpr size_t PERF_EVENT_ATTR_SAMPLE_REGS_USER_OFF = 80;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_THROUGH_SAMPLE_REGS_USER = 88;
> +constexpr size_t PERF_EVENT_ATTR_SAMPLE_REGS_INTR_OFF = 96;
> +constexpr size_t PERF_EVENT_ATTR_MIN_BYTES_THROUGH_SAMPLE_REGS_INTR = 104;
Why not just use the standard struct from perf_event.h? You would need to stop
using the accessors but they seem to be useless anyways. Or if you want
static offsets use offsetof
> +{
> + if (mapped_file_.size () < sizeof (header))
> + {
> + fprintf (stderr, _("File too small to contain perf header\n"));
> + return false;
> + }
> +
> + std::memcpy (&header, mapped_file_.data (), sizeof (header));
> +
> + if (header.magic != PERF_MAGIC)
> + {
> + if (header.magic == PERF_MAGIC_SWAPPED)
> + fprintf (stderr,
> + _("Error: big-endian perf.data is not supported\n"));
> + else
> + fprintf (stderr,
> + _("Invalid perf.data magic number: 0x%" PRIx64 "\n"),
> + header.magic);
> + return false;
> + }
> +
> + const uint8_t *const base = mapped_file_.data ();
I think i mentioned this before but not supporting pipe mode here is a
big loss for long profiles. Also with the old autofdo we've had
a lot of problems with running out of memory for long profiles (that is why gcc
autoprofiledbootstrap runs with such a slow period). Maybe the
MADV_SEQUENTIAL mitigates it, but I'm not sure.
At least for the bulk of the data it would be better to just use streaming
to side step all of this.
Okay you have a live collector with can avoid some of this,
but there are many use cases where perf is also useful.
> + periods_differ = true;
> + ++sampling_attrs;
> + chosen = layout;
> + }
> +
> + sample_layout = chosen;
> + /* TODO: weight each sample by its own event's period. The events are
> + told apart by the id that PERF_SAMPLE_IDENTIFIER puts in every
> + sample. */
> + if (periods_differ)
> + fprintf (stderr,
> + _("Warning: perf.data records sampling events with "
> + "different periods; samples are still counted as equal "
> + "mass\n"));
Why are you weighting samples? You cannot tell anything about
the branches not sampled anyways, and autofdo is just about
branchers. Do I miss something?
Also throttling can always happen.
> +
> + /* PERF_RECORD_LOST is { u64 id; u64 lost; }; PERF_RECORD_LOST_SAMPLES
> + is { u64 lost; }. */
> + if (record_header.type == perf_record_type::LOST
> + && payload_size >= 2 * sizeof (uint64_t))
> + lost += read_u64 (payload_ptr + sizeof (uint64_t));
> + else if (record_header.type == perf_record_type::LOST_SAMPLES
> + && payload_size >= sizeof (uint64_t))
> + lost += read_u64 (payload_ptr);
> +
> + local_ptr += skip;
> + local_rem -= skip;
> + fn (record_header, payload_ptr, payload_size, skip);
> + }
Would be good to tell the user about losses.
> + _("Error: perf.data changes the executable mapping lifetime for "
> + "a target process; this is not supported\n"));
> + /* TODO: Use sample_id_all timestamps and ordered per-PID mapping history
> + to support exec, unmap, PID reuse and cross-CPU record reordering. */
Hmm, I surprised that a parallel autoprofiledbootstrap works without
that.
> +/* Match a mapping against the profiled binary, by build ID where one is
> + known and by basename otherwise. */
> +bool
> +perf_file_reader::is_target_binary (const std::string &mmap_filename,
> + const std::string &build_id) const
> +{
> + if (mmap_filename.empty ())
> + return false;
> + const std::string mmap_path = normalize_mmap_filename (mmap_filename);
> +
> + if (!cached_target_build_id_valid_)
> + {
> + cached_target_build_id_ = get_buildid_from_binary (binary_path);
> + cached_target_build_id_valid_ = true;
> + }
> + if (!buildid_table_valid_)
> + parse_build_id_table ();
> +
> + size_t tp = binary_path.find_last_of ("/\\");
> + std::string target_basename
> + = (tp != std::string::npos) ? binary_path.substr (tp + 1) : binary_path;
> + size_t mp = mmap_path.find_last_of ("/\\");
> + std::string mmap_basename
> + = (mp != std::string::npos) ? mmap_path.substr (mp + 1) : mmap_path;
> + const bool basename_matches = mmap_basename == target_basename;
> +
> + /* Prefer the build ID from MMAP2, else the one HEADER_BUILD_ID recorded
> for
> + this path, which also covers a renamed or relocated binary. */
> + std::string recorded_build_id = build_id;
> + if (recorded_build_id.empty ())
> + {
> + const auto it = buildid_table_.find (mmap_path);
> + if (it != buildid_table_.end ())
> + recorded_build_id = it->second;
> + }
> +
> + /* A recorded build ID decides the match whenever the requested binary has
> + one too; falling back to the basename would accept a different build.
> */
Accepting different binaries is a important feature for autofdo.
For contiguous integration you can build with last weeks profile data.
So that means that if both have build ids this would be rejected?
> + recorded_build_id.c_str (),
> + binary_path.c_str (),
> + cached_target_build_id_.c_str ());
> +}
> +
> +/* Reject kernel and very low addresses. */
> +bool
> +perf_file_reader::is_valid_user_address (uint64_t addr) const
> +{
> + const bool is_kernel = (addr & KERNEL_ADDRESS_MASK) == KERNEL_ADDRESS_MASK;
> + return addr >= MIN_USER_ADDRESS && !is_kernel;
> +}
But we eventually want to support the kernel too. Hopefully it just
happens naturally because you need to already reject JITed code
efficiently, so you don't need this special check.
