Skip to content
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

New function to log object graph #20

Merged
merged 17 commits into from
Sep 12, 2024

Conversation

beta-ziliani
Copy link
Member

Produces beautiful graphs like:

graph LR
  0x101832e80["0x101832e80 C"]
  0x101832e70["0x101832e70 C"]
  0x10182be80["0x10182be80 A"] --@c,0--> 0x101832e80
  0x10182be40["0x10182be40 B"] --@c,0--> 0x101832e80
  0x10182be20["0x10182be20 (class 0)"] --@(field 0),1--> 0x10182be40
  0x10182be60["0x10182be60 Array(B)"] --@(field 16),2--> 0x10182be20
  0x10182be80["0x10182be80 A"] --@bs,3--> 0x10182be60
  0x10182be00["0x10182be00 B"] --@c,0--> 0x101832e70
  0x10182be20["0x10182be20 (class 0)"] --@(field 8),1--> 0x10182be00
Loading

Copy link
Contributor

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

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

I left a suggestion to reduce avoid some allocations (if manipulating lots of data), and maybe we can optimize a couple more things later.

Another suggestion would be to split building the stack in one method, from generating the mermaid graph in another (maybe).

src/perf_tools/mem_prof.cr Outdated Show resolved Hide resolved
@beta-ziliani
Copy link
Member Author

Another suggestion would be to split building the stack in one method, from generating the mermaid graph in another (maybe).

Yes, this is an issue with the tool, see #21 .

@beta-ziliani
Copy link
Member Author

Addressing comments from core review

core review: it is said of a code review from a core team member 🤭

@beta-ziliani beta-ziliani merged commit bf884bc into crystal-lang:main Sep 12, 2024
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