Skip to content

Conversation

ilumsden
Copy link
Collaborator

This PR adds performance measurement annotations to Thicket using the APIs added to Hatchet in LLNL/hatchet#142.

@ilumsden ilumsden added area-utils Issues and PRs related to Thicket's internal utilities and helpers priority-normal Normal priority issues and PRs status-ready-for-review This PR is ready to be reviewed by assigned reviewers type-feature Requests for new features or PRs which implement new features labels Jul 14, 2024
@ilumsden ilumsden self-assigned this Jul 14, 2024
Copy link
Collaborator

@dyokelson dyokelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilumsden Can you remind me - is there anything important about the order of the decorators or are they applied independently? Just wondering what the outcome will be for some functions with the cache_stats_op and perf annotations.

Otherwise looks good, since this is just annotations and the functionality is in hatchet.

Copy link
Collaborator

@michaelmckinsey1 michaelmckinsey1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments:

  1. Is it possible to add metadata with the pycaliper interface?
  2. I think the amount of annotations is causing high overhead. See following:

Time to create Thicket with X amount of files.

files pycaliper develop
1,000 36s 8s
10,000 15m36s 1m26s

I notice Calls for Node.__eq__ is 96,174,300 for the thicket with 10,000 files. By removing all of the node annotations in Hatchet for pycaliper time goes from 15m36s to 3m. So I think we may have to be more selective with what we annotate or maybe provide some kind of different levels for profiling.

@ilumsden
Copy link
Collaborator Author

Thanks for taking a look @michaelmckinsey1. That timing info is very interesting. I definitely agree that the annotations need to be paired down given the timing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-utils Issues and PRs related to Thicket's internal utilities and helpers priority-normal Normal priority issues and PRs status-ready-for-review This PR is ready to be reviewed by assigned reviewers type-feature Requests for new features or PRs which implement new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants