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

Infra for mmap memory backed by temporary files #7262

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

mpirvu
Copy link
Contributor

@mpirvu mpirvu commented Feb 9, 2024

This commit adds support for allocating memory using mmap and backing that memory by a temporary file.
The goal is to reduce the amount of physical memory in use by saving the content of an infrequently used memory segment to its backing file using the madvise() system call with a hint of MADV_PAGEOUT.
This feature is only available for Linux.

@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 9, 2024

@babsingh Could you please review this portlib PR or delegate? Thanks

port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
include_core/j9nongenerated.h Outdated Show resolved Hide resolved
port/common/omrport.tdf Outdated Show resolved Hide resolved
@babsingh
Copy link
Contributor

babsingh commented Feb 9, 2024

@keithc-ca Can you please review this PR as well?

@mpirvu mpirvu force-pushed the disclaim branch 2 times, most recently from a36dd9d to ecddbfc Compare February 9, 2024 16:36
@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 9, 2024

@babsingh I have addressed all your suggestions from the review.
Note that I have added the new trace point at the very end of the tdf file, but that broke the nice grouping by topic that existed in the tdf file.

include_core/j9nongenerated.h Outdated Show resolved Hide resolved
include_core/omrport.h Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 9, 2024

@keithc-ca I have addressed all your comments. The PR is ready for another review. Thanks

@babsingh
Copy link
Contributor

babsingh commented Feb 9, 2024

I have addressed all your suggestions from the review.

Thanks.

Note that I have added the new trace point at the very end of the tdf file, but that broke the nice grouping by topic that existed in the tdf file.

Yes, it is a drawback. Currently, we can't preserve the grouping by topic and numbering simultaneously. Preserving tracepoint numbering takes precedence to avoid customer issues.

port/linux/omrvmem.c Outdated Show resolved Hide resolved
Copy link
Member

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

This looks good, with a suggestion for making it better still.

@babsingh
Copy link
Contributor

babsingh commented Feb 9, 2024

jenkins build all

@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 9, 2024

The latest push introduces a subtle change: previously, if we failed to open our file, we attempt to use an anonymous map with this code:

		if (OMRPORT_INVALID_FD == fd) {
			/* Revert to using an ANONYMOUS mmap. */
			useBackingSharedTmpFile = FALSE;
			flags = 0;
			useBackingFile = set_flags_for_mmap(&flags);
			Trc_PRT_vmem_reserve_tempfile_not_created(filename, byteAmount);
		}

As long as MAP_ANONYMOUS or MAP_ANON are defined on the system, everything is good. However if MAP_ANONYMOUS and MAP_ANON are not defined, then we try to use a mmap operation with /dev/zero as file and set useBackingFile to TRUE. In this case we need to open that /dev/zero file and have a valid file descriptor, which was not happening. The older code above mine (the one for useBackingSharedFile) had the same flaw.
The change I introduced was to delete an else statement so that we execute the following check again

	if (useBackingFile) {
		fd = portLibrary->file_open(portLibrary, "/dev/zero", EsOpenRead | EsOpenWrite, 0);
	}

port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
@mpirvu mpirvu force-pushed the disclaim branch 2 times, most recently from f650946 to bc8d5af Compare February 9, 2024 22:53
port/linux/omrvmem.c Outdated Show resolved Hide resolved
include_core/omrport.h Outdated Show resolved Hide resolved
port/linux/omrvmem.c Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/aix/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/ztpf/omrvmem.c Outdated Show resolved Hide resolved
port/linux/omrvmem.c Outdated Show resolved Hide resolved
port/zos390/omrvmem.c Outdated Show resolved Hide resolved
This commit adds support for allocating memory using
mmap and backing that memory by a temporary file.
The goal is to reduce the amount of physical memory in use
by saving the content of an infrequently used memory segment
to its backing file using the madvise() system call with
a hint of MADV_PAGEOUT.
This feature is only available for Linux.

Signed-off-by: Marius Pirvu <[email protected]>
@babsingh
Copy link
Contributor

jenkins build all

@babsingh
Copy link
Contributor

jenkins build win

Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

OSX PR build failed due to a known and unrelated issue: #7181.

@babsingh babsingh merged commit 6ef9906 into eclipse:master Feb 15, 2024
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants