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

Hide BeginNode/EndNode and use a finer-grained coloring scheme for effect nodes #59

Merged
merged 3 commits into from
Aug 26, 2022

Conversation

eregon
Copy link
Contributor

@eregon eregon commented Aug 26, 2022

Fixes #45.

examples/ruby/example_instance_of.bgv:5:
Before:
old
After:
new


examples/ruby/example_while_break.bgv:5:
Before:
old
After:
new


examples/graalvm-ce-java11-21.2.0/fib-ruby.bgv.gz:3:
Before:
old
After:
new


"Empty if":
Before:
old
After:
new

@eregon
Copy link
Contributor Author

eregon commented Aug 26, 2022

I added svg graphs to compare before/after.

@eregon
Copy link
Contributor Author

eregon commented Aug 26, 2022

In mid-tier we have BeginNodes with multiple output edges:
Screenshot from 2022-08-26 17-01-27

Should we leave those in the graph?
To remove them we'd need more logic to choose which edge properties to prefer.
And also then those anchor would be on the if and not only on the Begin of the F (false) branch.
So I guess better leave them in if multiple inputs or outputs edges.

* Split "effect" into memory, call, alloc, sync.
* Use a softer color for memory.
@chrisseaton
Copy link
Contributor

Leave them in unless they're very simple.

@chrisseaton
Copy link
Contributor

Not convinced about this purple but I'll merge it for now.

@chrisseaton chrisseaton merged commit 41f6278 into Shopify:master Aug 26, 2022
@eregon
Copy link
Contributor Author

eregon commented Aug 26, 2022

Not convinced about this purple but I'll merge it for now.

Feel free to tweak. I think I'll take anything except red for memory accesses, because red is already used for control flow and control flow is completely unrelated to memory accesses.

@chrisseaton
Copy link
Contributor

control flow is completely unrelated to memory accesses

Both are usually fixed, that's why.

@eregon eregon deleted the nicer-graphs branch August 26, 2022 16:10
@eregon eregon restored the nicer-graphs branch August 26, 2022 16:10
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems January 4, 2023 14:55 Inactive
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.

Less aggresive color for loads and side effects
2 participants