Skip to content

Commit

Permalink
remove default constructors of Events, use std::optional instead
Browse files Browse the repository at this point in the history
  • Loading branch information
Tessa Todorowski committed Nov 12, 2024
1 parent 966a1fc commit b87b684
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 54 deletions.
16 changes: 8 additions & 8 deletions include/lo2s/perf/bio/writer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class Writer

void write(IoReaderIdentity& identity, TracepointSampleType* header)
{
if (identity.tracepoint == bio_queue_)
if (identity.tracepoint() == bio_queue_)
{
struct RecordBioQueue* event = (RecordBioQueue*)header;

Expand Down Expand Up @@ -112,7 +112,7 @@ class Writer
time_converter_(event->header.time), handle, mode,
otf2::common::io_operation_flag_type::non_blocking, size, event->sector);
}
else if (identity.tracepoint == bio_issue_)
else if (identity.tracepoint() == bio_issue_)
{
struct RecordBlock* event = (RecordBlock*)header;

Expand All @@ -129,7 +129,7 @@ class Writer
writer << otf2::event::io_operation_issued(time_converter_(event->header.time), handle,
event->sector);
}
else if (identity.tracepoint == bio_complete_)
else if (identity.tracepoint() == bio_complete_)
{
struct RecordBlock* event = (RecordBlock*)header;

Expand All @@ -150,7 +150,7 @@ class Writer
}
else
{
throw std::runtime_error("tracepoint " + identity.tracepoint.name() +
throw std::runtime_error("tracepoint " + identity.tracepoint().name() +
" not valid for block I/O");
}
}
Expand All @@ -164,7 +164,7 @@ class Writer
bio_complete_ =
perf::EventProvider::instance().create_tracepoint_event("block:block_rq_complete");

return { bio_queue_, bio_issue_, bio_complete_ };
return { bio_queue_.value(), bio_issue_.value(), bio_complete_.value() };
}

private:
Expand All @@ -185,9 +185,9 @@ class Writer
time::Converter& time_converter_;

// Unavailable until get_tracepoints() is called
perf::tracepoint::TracepointEvent bio_queue_;
perf::tracepoint::TracepointEvent bio_issue_;
perf::tracepoint::TracepointEvent bio_complete_;
std::optional<perf::tracepoint::TracepointEvent> bio_queue_;
std::optional<perf::tracepoint::TracepointEvent> bio_issue_;
std::optional<perf::tracepoint::TracepointEvent> bio_complete_;

// The unit "sector" is always 512 bit large, regardless of the actual sector size of the device
static constexpr int SECTOR_SIZE = 512;
Expand Down
16 changes: 11 additions & 5 deletions include/lo2s/perf/counter/counter_collection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include <lo2s/perf/event.hpp>

#include <optional>
#include <vector>

namespace lo2s
Expand All @@ -33,24 +34,29 @@ namespace counter
{
struct CounterCollection
{
Event leader;
std::optional<Event> leader_;
std::vector<Event> counters;

double get_scale(int index) const
{
if (index == 0)
{
return leader.scale();
return leader_.value().scale();
}
else
{
return counters[index - 1].scale();
}
}

Event& leader() const
{
return const_cast<Event&>(leader_.value());
}

friend bool operator==(const CounterCollection& lhs, const CounterCollection& rhs)
{
if (lhs.leader == rhs.leader)
if (lhs.leader_.value() == rhs.leader_.value())
{
return lhs.counters == rhs.counters;
}
Expand All @@ -59,11 +65,11 @@ struct CounterCollection

friend bool operator<(const CounterCollection& lhs, const CounterCollection& rhs)
{
if (lhs.leader == rhs.leader)
if (lhs.leader_.value() == rhs.leader_.value())
{
return lhs.counters < rhs.counters;
}
return lhs.leader < rhs.leader;
return lhs.leader_.value() < rhs.leader_.value();
}
};

Expand Down
3 changes: 2 additions & 1 deletion include/lo2s/perf/counter/counter_provider.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <lo2s/perf/counter/counter_collection.hpp>
#include <lo2s/perf/tracepoint/event.hpp>

#include <optional>
#include <vector>

namespace lo2s
Expand Down Expand Up @@ -58,7 +59,7 @@ class CounterProvider
std::vector<std::string> get_tracepoint_event_names();

private:
Event group_leader_;
std::optional<Event> group_leader_;
std::vector<Event> group_events_;
std::vector<Event> userspace_events_;
std::vector<tracepoint::TracepointEvent> tracepoint_events_;
Expand Down
3 changes: 1 addition & 2 deletions include/lo2s/perf/event.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ class Event
public:
Event(const std::string& name, perf_type_id type, std::uint64_t config,
std::uint64_t config1 = 0);
Event();

/**
* returns an opened instance of any Event object
Expand Down Expand Up @@ -214,9 +213,9 @@ class Event
class SysfsEvent : public Event
{
public:
using Event::Event;
SysfsEvent(const std::string& ev_name, bool enable_on_exec = false);

void make_invalid();
void use_sampling_options(const bool& use_pebs, const bool& sampling, const bool& enable_cct);
};

Expand Down
15 changes: 10 additions & 5 deletions include/lo2s/perf/io_reader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,22 @@ struct IoReaderIdentity
{
IoReaderIdentity(std::string tracepoint_name, Cpu cpu) : cpu(cpu)
{
tracepoint = EventProvider::instance().create_tracepoint_event(tracepoint_name);
tracepoint_.value() = EventProvider::instance().create_tracepoint_event(tracepoint_name);
}

tracepoint::TracepointEvent tracepoint;
std::optional<tracepoint::TracepointEvent> tracepoint_;
Cpu cpu;

tracepoint::TracepointEvent tracepoint()
{
return tracepoint_.value();
}

friend bool operator>(const IoReaderIdentity& lhs, const IoReaderIdentity& rhs)
{
if (lhs.cpu == rhs.cpu)
{
return lhs.tracepoint > rhs.tracepoint;
return lhs.tracepoint_.value() > rhs.tracepoint_.value();
}

return lhs.cpu > rhs.cpu;
Expand All @@ -83,7 +88,7 @@ struct IoReaderIdentity
{
if (lhs.cpu == rhs.cpu)
{
return lhs.tracepoint < rhs.tracepoint;
return lhs.tracepoint_.value() < rhs.tracepoint_.value();
}

return lhs.cpu < rhs.cpu;
Expand All @@ -97,7 +102,7 @@ class IoReader : public PullReader
{
try
{
event_ = identity_.tracepoint.open(identity.cpu);
event_ = identity_.tracepoint().open(identity.cpu);
}
catch (const std::system_error& e)
{
Expand Down
4 changes: 0 additions & 4 deletions include/lo2s/perf/tracepoint/event.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ class TracepointEvent : public Event
ParseError(const std::string& what, int error_code);
};

TracepointEvent() : id_(-1)
{
}

TracepointEvent(const std::string& name, bool enable_on_exec = false);

void parse_format();
Expand Down
2 changes: 1 addition & 1 deletion include/lo2s/trace/trace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ class Trace

if (scope.type == MeasurementScopeType::GROUP_METRIC)
{
metric_class.add_member(get_event_metric_member(counter_collection.leader));
metric_class.add_member(get_event_metric_member(counter_collection.leader()));
}

for (const auto& counter : counter_collection.counters)
Expand Down
8 changes: 4 additions & 4 deletions src/perf/counter/counter_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ void CounterProvider::initialize_group_counters(const std::string& leader,
try
{
// skip event if it has already been declared as group leader
if (ev == group_leader_.name())
if (ev == group_leader_.value().name())
{
Log::info() << "'" << ev
<< "' has been requested as both the metric leader event and a regular "
Expand Down Expand Up @@ -150,9 +150,9 @@ CounterCollection CounterProvider::collection_for(MeasurementScope scope)
CounterCollection res;
if (scope.type == MeasurementScopeType::GROUP_METRIC)
{
if (group_leader_.is_available_in(scope.scope))
if (group_leader_.value().is_available_in(scope.scope))
{
res.leader = group_leader_;
res.leader() = group_leader_.value();
for (auto& ev : group_events_)
{
if (ev.is_available_in(scope.scope))
Expand Down Expand Up @@ -219,7 +219,7 @@ bool CounterProvider::has_group_counters(ExecutionScope scope)
}
else
{
return group_leader_.is_available_in(scope) &&
return group_leader_.value().is_available_in(scope) &&
std::any_of(group_events_.begin(), group_events_.end(),
[scope](const auto& ev) { return ev.is_available_in(scope); });
}
Expand Down
16 changes: 9 additions & 7 deletions src/perf/counter/group/reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,27 @@ Reader<T>::Reader(ExecutionScope scope, bool enable_on_exec)
{
if (config().metric_use_frequency)
{
counter_collection_.leader.sample_freq(config().metric_frequency);
counter_collection_.leader().sample_freq(config().metric_frequency);
}
else
{
counter_collection_.leader.sample_period(config().metric_count);
counter_collection_.leader().sample_period(config().metric_count);
}

do
{
try
{
counter_leader_ =
counter_collection_.leader.open_as_group_leader(scope, config().cgroup_fd);
counter_collection_.leader().open_as_group_leader(scope, config().cgroup_fd);
}
catch (const std::system_error& e)
{
// perf_try_event_open was used here before
if (counter_leader_.value().get_fd() < 0 && errno == EACCES &&
!counter_collection_.leader.attr().exclude_kernel && perf_event_paranoid() > 1)
!counter_collection_.leader().attr().exclude_kernel && perf_event_paranoid() > 1)
{
counter_collection_.leader.mut_attr().exclude_kernel = 1;
counter_collection_.leader().mut_attr().exclude_kernel = 1;
perf_warn_paranoid();

continue;
Expand All @@ -89,14 +89,16 @@ Reader<T>::Reader(ExecutionScope scope, bool enable_on_exec)
}
} while (!counter_leader_.value().is_valid());

Log::debug() << "counter::Reader: leader event: '" << counter_collection_.leader.name() << "'";
Log::debug() << "counter::Reader: leader event: '" << counter_collection_.leader().name()
<< "'";

for (auto& counter_ev : counter_collection_.counters)
{
if (counter_ev.is_available_in(scope))
{
std::optional<EventGuard> counter = std::nullopt;
counter_ev.mut_attr().exclude_kernel = counter_collection_.leader.attr().exclude_kernel;
counter_ev.mut_attr().exclude_kernel =
counter_collection_.leader().attr().exclude_kernel;

try
{
Expand Down
21 changes: 7 additions & 14 deletions src/perf/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,6 @@ static constexpr std::uint64_t apply_mask(std::uint64_t value, std::uint64_t mas
return res;
}

Event::Event() : name_("")
{
memset(&attr_, 0, sizeof(attr_));
attr_.size = sizeof(attr_);
attr_.type = PERF_TYPE_RAW;

attr_.config = 0;
attr_.config1 = 0;

attr_.sample_period = 0;
attr_.exclude_kernel = 1;
}

Event::Event(const std::string& name, perf_type_id type, std::uint64_t config,
std::uint64_t config1)
: name_(name)
Expand Down Expand Up @@ -361,7 +348,8 @@ bool Event::degrade_precision()
}
}

SysfsEvent::SysfsEvent(const std::string& ev_name, bool enable_on_exec) : Event()
SysfsEvent::SysfsEvent(const std::string& ev_name, bool enable_on_exec)
: Event(ev_name, static_cast<perf_type_id>(0), 0)
{
set_common_attrs(enable_on_exec);

Expand Down Expand Up @@ -483,6 +471,11 @@ SysfsEvent::SysfsEvent(const std::string& ev_name, bool enable_on_exec) : Event(
}
}

void SysfsEvent::make_invalid()
{
availability_ = Availability::UNAVAILABLE;
}

void SysfsEvent::use_sampling_options(const bool& use_pebs, const bool& sampling,
const bool& enable_cct)
{
Expand Down
5 changes: 4 additions & 1 deletion src/perf/event_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,10 @@ Event EventProvider::cache_event(const std::string& name)
catch (const InvalidEvent& e)
{
// emplace unavailable Sampling Event
event_map_.emplace(name, SysfsEvent());
SysfsEvent event = EventProvider::instance().create_sysfs_event(name, false);
event.make_invalid();

event_map_.emplace(name, event);
throw e;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/perf/tracepoint/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ namespace tracepoint
{

TracepointEvent::TracepointEvent(const std::string& name, bool enable_on_exec)
: Event(), name_(name)
: Event(name, PERF_TYPE_TRACEPOINT, 0), name_(name)
{
set_common_attrs(enable_on_exec);
parse_format();

// update to correct config (id_ set in parse_format())
attr_.config = id_;
attr_.type = PERF_TYPE_TRACEPOINT;
attr_.sample_type |= PERF_SAMPLE_RAW | PERF_SAMPLE_IDENTIFIER;

update_availability();
Expand Down

0 comments on commit b87b684

Please sign in to comment.