-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add named "additional data" support #121
base: master
Are you sure you want to change the base?
Conversation
This allows representing KV pairs of data in e.g. chrome profiles more faithfully.
356671f
to
48c55c4
Compare
Note: this doesn’t actually add the API to measureme to output such data. Ideas on what API would make most sense here would be appreciated. |
/// <argument> = ['\x1D' <argument_name>] '\x1E' <argument_value> | ||
/// <argument_name> = <text> | ||
/// <argument_value> = <text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be simpler to make the grammar:
<argument> = '\x1E' (<argument_name_value_pair> | <argument_value>)
<argument_name_value_pair> = <text> '\x1D' <text>
<argument_value> = <text>
Then we could always parse the leading \x1E
and then do a string split on the \x1D
if it exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would also make the format compatible with the current format. It's fine if we need to break that but we'll have to bump the format number and coordinate redeploying new versions on perf.rlo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like there might be another breaking change in the works from another contributor so if you think it's simpler to use the current strategy rather than this suggestion, that's fine with me!
I’m probably not going to pursue this further, but if anybody wants feel free to pick this up. |
This allows representing KV pairs of data in e.g. chrome profiles more
faithfully. Previously we would automatically generate arg0... as a key
for all such data.