Skip to content

Conversation

@ndh4
Copy link

@ndh4 ndh4 commented Sep 27, 2024

Adds the flag --instantiation-only to the raco contract-profile <flags> <filename> interface. When this flag is set, the work of visiting <filename> (which include visiting and instantiating all of the modules specified in <filename>'s require-forms) is left out of the profile.

@ndh4 ndh4 marked this pull request as ready for review September 27, 2024 19:00
@stamourv
Copy link
Contributor

stamourv commented Oct 2, 2024

Is the word "instantiation" really the most evocative word to use here?
I understand that this is the correct term in terms of the syntax model, but how meaningful is it to someone using the profiler? That is, how would someone know if they should use that option?

Since this is such a small change, it should really be a single commit; can you squash these 5 into 1?

Also, I assume you've tested it and it behaves as you expect?

Thanks!

@rfindler
Copy link
Member

rfindler commented Oct 2, 2024 via email

@rfindler
Copy link
Member

rfindler commented Oct 2, 2024

Ah, yes. I see docs. Sorry!

@rfindler
Copy link
Member

rfindler commented Oct 2, 2024

Also, I assume you've tested it and it behaves as you expect?

I'm a little puzzled by this too, actually. Just be reasoning about booleans and whatnot, the presence of the flag doesn't seem to affect the program.

@ndh4
Copy link
Author

ndh4 commented Oct 8, 2024

Is the word "instantiation" really the most evocative word to use here? I understand that this is the correct term in terms of the syntax model, but how meaningful is it to someone using the profiler? That is, how would someone know if they should use that option?

You're right that "instantiation" is overly technical, but in my perspective, this is a good thing. I don't want someone to be comfortable using this option unless they understand what it means, and I think knowing about the syntax model is a necessary condition for this understanding. If the user knows what "instantiation" means, they can derive the exact implications of how setting the flag would change the result. Naming the flag after one of those implications would be more evocative, but it would be less precise.

The other general naming route I was thinking of going was something like "--exclude-require-statements", "--no-imports", or "--this-module-only". But this would introduce an ambiguity that could mislead a user into thinking that only the module specified (which I will refer to as main in this paragraph) will be profiled when they use this flag. A user might imagine a profiler that stops profiling when it enters code that originated outside of main, and picks up where it left off once it re-enters code that originated from main. The names in this paragraph may lead the user to think that that is what happens when the flag is set.

If you have any other ideas for the naming, I would love to hear them. I think something like "--exclude-initial-visiting" might also work.

@ndh4
Copy link
Author

ndh4 commented Oct 8, 2024

Also, I assume you've tested it and it behaves as you expect?

I just added a test to make sure that --instantiation-only always returns a greater number for total time when the flag is not present than when it is. It isn't necessarily an always-true property (i.e. if the module loader is very, very fast, this property may not hold), but practically speaking, it should always be the case. Let me know if there's anything else you think I should check for.

@ndh4
Copy link
Author

ndh4 commented Oct 8, 2024

I'm a little puzzled by this too, actually. Just be reasoning about booleans and whatnot, the presence of the flag doesn't seem to affect the program.

module-to-profile calls dynamic-require on the module it is given, which causes the module to be visited for the first time. So, in raco.rkt, the location where module-to-profile is called is the location of the inital visit to that module. When instantiation-only? is true, this initial visit happens in line 38, outside the scope of the contract-profile call, meaning that the initial visit is not profiled. When instantiation-only? is false, this initial visit happens in line 49, as part of the expression given to contract-profile, meaning that the initial visit is profiled.

@stamourv
Copy link
Contributor

Re instantiation: I see what you mean. But even for someone who understands what instantiation means, it's not obvious to me that they'd know when they want to use the flag. Could the name be connected to the use case, rather than to the mechanism used to implement it?

@rfindler
Copy link
Member

I'm a little puzzled by this too, actually. Just be reasoning about booleans and whatnot, the presence of the flag doesn't seem to affect the program.

module-to-profile calls dynamic-require on the module it is given, which causes the module to be visited for the first time. So, in raco.rkt, the location where module-to-profile is called is the location of the inital visit to that module. When instantiation-only? is true, this initial visit happens in line 38, outside the scope of the contract-profile call, meaning that the initial visit is not profiled. When instantiation-only? is false, this initial visit happens in line 49, as part of the expression given to contract-profile, meaning that the initial visit is profiled.

Oh, right. Sorry!

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