Skip to content

Infra for mmap memory backed by temporary files - #7262

Merged
babsingh merged 1 commit into
eclipse-omr:masterfrom
mpirvu:disclaim
Feb 15, 2024
Merged

Infra for mmap memory backed by temporary files#7262
babsingh merged 1 commit into
eclipse-omr:masterfrom
mpirvu:disclaim

Conversation

@mpirvu

@mpirvu mpirvu commented Feb 9, 2024

Copy link
Copy Markdown
Contributor

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

mpirvu commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

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

Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread include_core/j9nongenerated.h Outdated
Comment thread port/common/omrport.tdf Outdated
@babsingh

babsingh commented Feb 9, 2024

Copy link
Copy Markdown
Contributor

@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

mpirvu commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

@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.

Comment thread include_core/j9nongenerated.h Outdated
Comment thread include_core/omrport.h
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
@mpirvu

mpirvu commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

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

@babsingh

babsingh commented Feb 9, 2024

Copy link
Copy Markdown
Contributor

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.

Comment thread port/linux/omrvmem.c Outdated

@keithc-ca keithc-ca left a comment

Copy link
Copy Markdown
Contributor

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

babsingh commented Feb 9, 2024

Copy link
Copy Markdown
Contributor

jenkins build all

@mpirvu

mpirvu commented Feb 9, 2024

Copy link
Copy Markdown
Contributor Author

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);
	}

Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
@mpirvu
mpirvu force-pushed the disclaim branch 2 times, most recently from f650946 to bc8d5af Compare February 9, 2024 22:53
Comment thread port/linux/omrvmem.c Outdated
Comment thread include_core/omrport.h Outdated
Comment thread port/linux/omrvmem.c
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/aix/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/ztpf/omrvmem.c Outdated
Comment thread port/linux/omrvmem.c Outdated
Comment thread port/zos390/omrvmem.c Outdated
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 <mpirvu@ca.ibm.com>
Comment thread include_core/j9nongenerated.h
@babsingh

Copy link
Copy Markdown
Contributor

jenkins build all

@babsingh

Copy link
Copy Markdown
Contributor

jenkins build win

@babsingh babsingh left a comment

Copy link
Copy Markdown
Contributor

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.

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.

4 participants