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

Switch from 2GB memory space to 3/4 of physical memory #3483

Merged
merged 2 commits into from
Jun 14, 2019

Conversation

ChrisJefferson
Copy link
Contributor

@ChrisJefferson ChrisJefferson commented Jun 1, 2019

The current 2GB memory limit was set in 2012, and memory has grown since then.

On 64-bit OSes, this switches to a default of 3/4 of physical memory, or 2GB, whichever is larger. 32-bit OSes are left at their previous 1GB (just because I don't want to think about issues of filling up the memory space on 32-bit OSes, and most people won't be using them any more).

Tested on linux, mac and cygwin.

For release notes:

  • Changed the pre-set memory limit default from 2GB to 3/4 of physical memory. Use the '-o' option if you want to change this limit.

@ChrisJefferson ChrisJefferson force-pushed the physical-mem branch 2 times, most recently from 4e14cfe to bc0d327 Compare June 1, 2019 19:50
@codecov
Copy link

codecov bot commented Jun 1, 2019

Codecov Report

Merging #3483 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3483      +/-   ##
==========================================
- Coverage   85.43%   85.37%   -0.07%     
==========================================
  Files         699      698       -1     
  Lines      346759   344604    -2155     
==========================================
- Hits       296253   294204    -2049     
+ Misses      50506    50400     -106
Impacted Files Coverage Δ
src/system.c 76.1% <100%> (+2.32%) ⬆️
src/modules.h 33.33% <0%> (-66.67%) ⬇️
src/cyclotom.h 50% <0%> (-50%) ⬇️
src/records.h 50% <0%> (-50%) ⬇️
src/gapstate.h 42.85% <0%> (-42.86%) ⬇️
src/vec8bit.h 51.78% <0%> (-42.86%) ⬇️
src/macfloat.h 57.14% <0%> (-42.86%) ⬇️
lib/gprdpc.gi 45.74% <0%> (-42.2%) ⬇️
src/code.h 57.14% <0%> (-40.16%) ⬇️
src/gvars.h 66.66% <0%> (-33.34%) ⬇️
... and 115 more

This is required to let GAP dynamically calculate these
values, based on the user's physical memory
@coveralls
Copy link

coveralls commented Jun 1, 2019

Coverage Status

Coverage increased (+0.0003%) to 85.259% when pulling cf43990 on ChrisJefferson:physical-mem into a7e6a89 on gap-system:master.

Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

Do you think it would be a good idea to document (perhaps with the command line option for -o) what the defaults are? Not a requirement.

src/system.c Show resolved Hide resolved
src/system.c Outdated Show resolved Hide resolved
@wilfwilson wilfwilson added kind: general proposed change release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: kernel labels Jun 5, 2019
Copy link
Contributor

@hulpke hulpke left a comment

Choose a reason for hiding this comment

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

I think it is fantastic to be able to get the available memory -- in fact I would love in the library to have the abilikty to poll the available memory to check wehther certain calculations are possible.

Setting the default to 3/4 available memoryt could be an issue on multi-user systems (and that was the initial reason for setting a limit): A careless beginner can easily start allocating enormous amounts of memory, by asking for the elements of S_50 or so. How about maintaining a safety level at what would be above a single-user system, but could be on a multi-user server -- somewhere around 16GB?

@ChrisJefferson ChrisJefferson merged commit ff93de2 into gap-system:master Jun 14, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Jun 16, 2019
@ChrisJefferson ChrisJefferson deleted the physical-mem branch July 1, 2019 21:15
@wucas wucas added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: general proposed change release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants