Skip to content

Commit

Permalink
Use stable sort for performance entries (#36998)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #36998

For performance entries that have the same start/end time it makes more sense to report them back to `PerformanceObserver` in the same order they were logged.

This kind of determinism is arguably better both in terms of API, and from the point of view of testing.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D45142500

fbshipit-source-id: 77ff0093bead45dc2f15efc4b903dc181927565a
  • Loading branch information
rshest authored and facebook-github-bot committed Apr 20, 2023
1 parent 1250b8a commit 8e046cb
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ GetPendingEntriesResult PerformanceEntryReporter::popPendingEntries() {
}

// Sort by starting time (or ending time, if starting times are equal)
std::sort(
std::stable_sort(
res.entries.begin(),
res.entries.end(),
[](const RawPerformanceEntry &lhs, const RawPerformanceEntry &rhs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) {
reporter.measure("measure3", 0.0, 0.0, 5.0, "mark1");
reporter.measure("measure4", 1.5, 0.0, std::nullopt, std::nullopt, "mark2");

reporter.mark("mark3", 2.0);
reporter.measure("measure5", 2.0, 2.0);
reporter.mark("mark4", 2.0);

auto res = reporter.popPendingEntries();
const auto &entries = res.entries;

Expand Down Expand Up @@ -207,7 +211,29 @@ TEST(PerformanceEntryReporter, PerformanceEntryReporterTestReportMeasures) {
0.0,
std::nullopt,
std::nullopt,
std::nullopt}};
std::nullopt},
{"mark3",
static_cast<int>(PerformanceEntryType::MARK),
2.0,
0.0,
std::nullopt,
std::nullopt,
std::nullopt},
{"mark4",
static_cast<int>(PerformanceEntryType::MARK),
2.0,
0.0,
std::nullopt,
std::nullopt,
std::nullopt},
{"measure5",
static_cast<int>(PerformanceEntryType::MEASURE),
2.0,
0.0,
std::nullopt,
std::nullopt,
std::nullopt},
};

ASSERT_EQ(expected, entries);
}
Expand Down

0 comments on commit 8e046cb

Please sign in to comment.