Skip to content

Comments

ProfilingEvaluator: update ExprBox.name to print builtin function and operator names#264

Merged
stephenamar-db merged 3 commits intodatabricks:masterfrom
JoshRosen:profile-nicer-output
Feb 6, 2025
Merged

ProfilingEvaluator: update ExprBox.name to print builtin function and operator names#264
stephenamar-db merged 3 commits intodatabricks:masterfrom
JoshRosen:profile-nicer-output

Conversation

@JoshRosen
Copy link
Contributor

@JoshRosen JoshRosen commented Jan 24, 2025

This PR implements a small quality-of-life improvement in ProfilingEvaluator, updating ExprBox.name so that it prints the names of built in functions and operators, making it easier to interpret profiler output.

For example, before we'd print:

[info] Profiling...
[info] Top 20 by time:
[info] - 1286ms ApplyBuiltin2 <some-path>/functions.jsonnet.TEMPLATE:162:15
[info] - 470ms ApplyBuiltin2 <some-path>/functions.jsonnet.TEMPLATE:152:15
[info] - 443ms BinaryOp <some-path>/functions.jsonnet.TEMPLATE:158:35
[info] - 307ms Lookup <some-path>/functions.jsonnet.TEMPLATE:158:42
[....]

With this PR's small change, that becomes:

[info] Top 20 by time:
[info] - 1161ms ApplyBuiltin2 (join) <some-path>/functions.jsonnet.TEMPLATE:162:15
[info] - 459ms ApplyBuiltin2 (join) <some-path>/functions.jsonnet.TEMPLATE:152:15
[info] - 439ms BinaryOp (+) <some-path>/functions.jsonnet.TEMPLATE:158:35
[info] - 311ms Lookup <some-path>/functions.jsonnet.TEMPLATE:158:42
[...]

@JoshRosen
Copy link
Contributor Author

Don't merge this until #265 lands because I'll need to update this for the ApplyBuiltIn3/4 added there.

@JoshRosen JoshRosen marked this pull request as draft January 24, 2025 22:04
@JoshRosen JoshRosen marked this pull request as ready for review January 27, 2025 21:40
@stephenamar-db stephenamar-db merged commit 9c7528a into databricks:master Feb 6, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants