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.

Sign up for GitHub

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

Jump to bottom

Add parameter to control warning about memory constraints #5162

Merged
merged 10 commits into from Sep 28, 2022
Merged

Add parameter to control warning about memory constraints #5162

merged 10 commits into from Sep 28, 2022

Conversation

kennknowles
Copy link
Contributor

@kennknowles kennknowles commented Jun 15, 2022 •
edited

Please take a look and see what you think about this approach. I did it pretty quick (in vim not even opening an IDE or compiling) so I'm leaning on CI and reviewer a bit here. Just wanted to get things started to make my suggestion very concrete.

Fixes #5159

@kennknowles kennknowles changed the title Add parameter to control warning about memory constraints (fixes #5159) Add parameter to control warning about memory constraints Jun 15, 2022
@kennknowles kennknowles reopened this Jun 15, 2022
@kennknowles kennknowles mentioned this pull request Jun 15, 2022
Closed
@mernst
Copy link
Member

mernst commented Jun 15, 2022

@kennknowles Do you prefer your name to be listed as "Kenn Knowles" or "Kenneth Knowles"?

@mernst
Copy link
Member

mernst commented Jun 15, 2022

I would like to find a better name than -AnoWarnMemoryConstraints if possible.

@kennknowles
Copy link
Contributor Author

Yea, fair point. Thanks for just jumping in and fixing it up!

@kennknowles
Copy link
Contributor Author

-AgcWarningLogLevel=NOTE could be more legible, but it seems like more of a knob than you need

@kennknowles
Copy link
Contributor Author

Opened plume-lib/plume-scripts#20 in preparation for adjusting contributors list.

@kennknowles
Copy link
Contributor Author

What do you think about these various options, or some other blend of choice of name for the parameter and choice of values (vs flag that does not accept a value)?

  • -AmemoryPressureLogLevel=<level>
  • -AdowngradeMemoryPressureLogLevel
  • -AwarnOnMemoryPressure=<boolean>

@kennknowles
Copy link
Contributor Author

Any thoughts on your preferred naming for the parameter?

smillst
smillst previously approved these changes Sep 28, 2022
View reviewed changes
Copy link
Member

@smillst smillst left a comment

Choose a reason for hiding this comment

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

The code changes look good to me. @mernst you wanted to find a better name. I haven't thought of any, should we just go withnoWarnMemoryConstraints?

@smillst smillst assigned mernst and unassigned smillst Sep 28, 2022
@mernst mernst enabled auto-merge (squash) September 28, 2022 15:42
@mernst
Copy link
Member

mernst commented Sep 28, 2022

I don't have a better suggestion for a flag name, so let's go with this.
Thanks, @kennknowles , and thanks for your patience also.

@mernst mernst merged commit 179a70e into typetools:master Sep 28, 2022
@kennknowles
Copy link
Contributor Author

Fab! Thank you!

@kennknowles kennknowles deleted the suppress-gc-warning branch September 29, 2022 00:44
wmdietl pushed a commit to eisop/checker-framework that referenced this pull request Oct 10, 2022
@kennknowles @wmdietl
Add parameter to control warning about memory constraints (typetools#…
4125656 
…5162)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smillst smillst smillst approved these changes

Assignees

@mernst mernst

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

-Werror interacts poorly with "Memory constraints are impeding performance; please increase max heap size"
3 participants
@kennknowles @mernst @smillst