[PROTON] Reduce msgpack serialization overhead-1#10393
Conversation
Jokeren
force-pushed
the
keren/proton-fast-file-write-base
branch
from
8ae066c to
f498c30
Compare
Jokeren
requested review from
antiagainst,
peterbell10,
ptillet and
zhanglx13
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ae066c2df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| MetricSummary metricSummary; | ||
| const std::map<MetricKind, std::unique_ptr<Metric>> emptyMetrics; | ||
| metricSummary.hasKernelMetric = true; |
There was a problem hiding this comment.
When a profile contains PC sampling or cycle metrics only on descendants/linked nodes, the root metrics map is counted and written before those children call packMetrics, so hasPCSamplingMetric/hasCycleMetric are still false and the required zero-valued root fields are omitted; conversely this line now emits kernel root fields even for profiles with no kernel metrics. The previous pre-order scan populated metricSummary from direct and linked metrics before serializing the root, which is needed for MsgPack to match the JSON Hatchet schema.
Useful? React with 👍 / 👎.
| writer.packMap( | ||
| countMetricEntries(treeNode.metricSet.metrics, isRoot) + | ||
| static_cast<uint32_t>(treeNode.metricSet.flexibleMetrics.size())); |
There was a problem hiding this comment.
For graph metric nodes that are direct children of the virtual root, upsertLinkedFlexibleMetric(..., linkedId) stores their values under that child id, and the JSON serializer promotes those values into the concrete node's metrics before appending the virtual child. This new map count only includes the concrete node's own flexible metrics, while packLinkedVirtualNode only promotes metrics from virtualNode.children, so linked flexible metrics attached to the virtual root children are dropped from MsgPack output for captured graph metrics.
Useful? React with 👍 / 👎.
…d buildHatchetMsgPack
…e unnecessary line breaks
Jokeren
force-pushed
the
keren/proton-fast-file-write-base
branch
from
f498c30 to
db814ea
Compare
2 participants
Use
string_viewandpackFixStrLiteral