Opened 12 years ago

Closed 9 years ago

#1746 closed bug (fixed)

Default swap file size doesn't enable Virtual Memory

Reported by: andreasf Owned by: leavengood
Priority: normal Milestone: R1
Component: Preferences/VirtualMemory Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: x86

Description

The VM preferences say I have 511.93 MB Physical Memory, setting the default Requested Swap File Size to 511 MB. With this default setting, Virtual Memory is disabled on reboot. Setting it to >= 512 MB works.

Seems like a rounding problem?

Attachments (3)

virtualmemory-0-size-fix.diff (441 bytes ) - added by hamish 9 years ago.
Fixes vm_size 0 behaviour described in #1746
virtualmemory-fixes.diff (6.6 KB ) - added by hamish 9 years ago.
Fixes more issues with VirtualMemory noted while fixing #1746
virtualmemory-changes.diff (18.2 KB ) - added by hamish 9 years ago.
UI support for changing the swap-file volume

Download all attachments as: .zip

Change History (17)

comment:1 by axeld, 12 years ago

I dunno what makes it work when setting it to a value over 512 MB, but virtual memory should never be enabled right now, since it's just not implemented yet :-)

comment:2 by stippi, 12 years ago

IIRC, Haiku does not really have support for paging out to a swap file, yet. Or does it? But that's a side note. The misbehavior you observed sounds still like a bug.

comment:3 by idefix, 11 years ago

It's not a rounding problem.

When you open the VirtualMemory application for the first time and only check the 'Enable Virtual Memory'-box (do not move the slider), the application will create (on close) a 'virtual_memory'-file with the following contents:

vm on
swap_size 0

The next time you open VirtualMemory, it will interpret this as Virtual Memory disabled.

comment:4 by idefix, 11 years ago

Type: enhancementbug

comment:5 by mmadia, 10 years ago

Any updates on this?

comment:6 by axeld, 10 years ago

Resolution: fixed
Status: newclosed

Yes, I guess this has been fixed in the mean time, thanks for the note.

in reply to:  3 comment:7 by idefix, 10 years ago

Resolution: fixed
Status: closedreopened

The bug is still there, only this time the default Requested Swap File Size is 1 MB.

See my previous comment for the cause of this bug. (The VirtualMemory application will only create a correct 'virtual_memory'-file when you have moved the slider with the mouse.)

by hamish, 9 years ago

Fixes vm_size 0 behaviour described in #1746

comment:8 by hamish, 9 years ago

Has a Patch: set

by hamish, 9 years ago

Attachment: virtualmemory-fixes.diff added

Fixes more issues with VirtualMemory noted while fixing #1746

comment:9 by leavengood, 9 years ago

Owner: changed from axeld to leavengood
Status: reopenedassigned

I applied hamish's first patch in hrev40056. Thanks hamish! And in an amazing coincidence he just added another patch. I'll check that out too.

comment:10 by hamish, 9 years ago

You're welcome.

That latest patch makes some changes suggested as by scottmc in the Google Code-In task for this bug. The changes aren't all bug-fixes in the purest sense, but I believe they improve functionality. Here's the details on what's changed, as copied from the Google page:

The size slider is now disabled when the checkbox is unticked. An error message is now given when there's no space for a swap file, and the UI is disabled accordingly. Cleaned up some redundant code.

There's also a bug whereby pressing the "defaults" button can force a swap size which is greater than the free space available (and also exceeds the range of the slider). Under this case, what should the swap size be set to instead? Perhaps half the available space?

comment:11 by leavengood, 9 years ago

I applied your latest patch in hrev40062. I removed the comment from Settings.cpp (though I do agree with it) and also changed the double assignment to zero on line 326 of SettingsWindow.cpp to two separate assignments. While that is valid C++ it violates our coding style. You have been added as Contributor in hrev40063.

Regarding your question: as I commented on Melange I think using half the space is probably a decent heuristic, but you may also want to see if that is very small in which case it is almost useless (much like the minimum size of 1 MB.) So in pseudo-code:

defaultSize = GetDefaultSize()
if (defaultSize > freeSpace) {
  defaultSize = freeSpace / 2
  if (defaultSize < 50 MB) { # I'm not sure what is best here
    # show error and disable virtual memory
  }
}

by hamish, 9 years ago

Attachment: virtualmemory-changes.diff added

UI support for changing the swap-file volume

comment:12 by hamish, 9 years ago

That patch completes the support for changing the swap volume on the UI side. It writes a "swap_volume <volume_name>" line to the settings file. There's a large amount of error checking in there too, so it's fairly robust. If you want to test it out, define SWAP_VOLUME_IMPLEMENTED at the top of Settings.h.

The default size problem has also been fixed. The code could do with some refactoring, and when I get the time I'll do that, but for now it's functional.

comment:13 by leavengood, 9 years ago

Thanks very much for the patch Hamish, it looks great. I made a few very small changes and committed it in hrev40101. Also for future reference when you add your copyright line you put it above older ones, at least in this style of headers where each person is listed. In the other style where Haiku, Inc has the copyright you list yourself alphabetically by last name. Maybe confusing but that is how it is ;)

comment:14 by leavengood, 9 years ago

Resolution: fixed
Status: assignedclosed

After all this I think we can close this old bug. Any other problems with this preflet should be logged in a new bug. For example if you want to refactor more Hamish just open a new bug. Thanks again for your work!

Note: See TracTickets for help on using tickets.