Skip to content

Asyncify verbose option#3022

Merged
kripken merged 7 commits intomasterfrom
advise
Aug 6, 2020
Merged

Asyncify verbose option#3022
kripken merged 7 commits intomasterfrom
advise

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Aug 4, 2020

This logs out the decisions made about instrumenting functions, which
can help figure out why a function is instrumented, or to get a list of
what might need to be.

As the test shows, it can print things like this:

[asyncify] import is an import that can change the state
[asyncify] calls-import can change the state due to import
[asyncify] calls-calls-import can change the state due to calls-import
[asyncify] calls-calls-calls-import can change the state due to calls-calls-import

(the test has calls-calls-calls-import => calls-calls-import => calls-import -> import).

Not sure how useful this might be, but it was easy to write up. Thoughts?

cc @curiousdannii @aardappel

Comment thread src/passes/Asyncify.cpp
std::cout << "[asyncify] " << func->name
<< "'s state is set based on the only-list to " << matched
<< '\n';
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is looks possible for some functions to be logged as added as can-change-state functions but if the only-list is present, they are eventually ignored silently, i.e., without any other log messages that they are ignored, right? Would it be better to check the only list when logging for other causes..?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this log message here would be shown, though, so it's not completely silent? This message would say they are set to false, which overrides the previous messages.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah you're right. Sorry I thought this is only printed when the value is true.

@aardappel
Copy link
Copy Markdown
Contributor

Yes, the "due to" will be super useful in tracking down why a function that doesn't appear necessary is included, thanks!

@kripken kripken merged commit e89b401 into master Aug 6, 2020
@kripken kripken deleted the advise branch August 6, 2020 16:27
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.

3 participants