gh-44098: Release the GIL during mmap on Unix #98146

hauntsaninja · 2022-10-10T09:49:41Z

This seems pretty straightforward. The issue mentions other calls in mmapmodule that we could release the GIL on, but those are in methods where we'd need to be careful to ensure that something sensible happens if those are called concurrently. In prior art, note that #12073 released the GIL for munmap. In a toy benchmark, I see the speedup you'd expect from doing this.

Issue: release GIL while doing I/O operations in the mmap module #44098

Automerge-Triggered-By: GH:gvanrossum

This seems pretty straightforward. The issue mentions other calls in mmapmodule that we could release the GIL on, but those are in methods where we'd need to be careful to ensure that something sensible happens if those are called concurrently. In prior art, note that python#12073 released the GIL for munmap. In a toy benchmark, I see the speed up you'd expect from doing this.

gvanrossum · 2022-10-10T20:47:42Z

Misc/NEWS.d/next/Library/2022-10-10-09-52-21.gh-issue-44098.okcqJt.rst

@@ -0,0 +1 @@
+Release the GIL when creating :class:`mmap.mmap` objects.


Might I suggest qualifying this with "on UNIX"? The Windows version is different, and I won't ask you to update it (we should leave that up to @zooba).

Otherwise seems harmless. Is this based on an actual use case?

Thanks, good suggestion, made the change.

This is not based on an actual use case. I was going through old issues and saw your comment asking for a patch here: #44098 (comment)
That said, I think I have some code at work that may benefit (would take some effort to figure out exactly how much though, and today's a holiday).

miss-islington · 2022-10-10T22:14:30Z

Status check is done, and it's a success ✅.

vstinner · 2022-10-10T22:16:15Z

In a toy benchmark, I see the speedup you'd expect from doing this.

Just curious: What is the benchmark? What are results?

hauntsaninja · 2022-10-10T22:25:04Z

Toy benchmark code:

import mmap
import time
import threading

with open("hello.txt", "wb") as f:
    f.write(b"Hello Python!\n")

with open("hello.txt", "r+b") as f:
    fn = f.fileno()

    def run_mmap():
        for _ in range(100):
            mmap.mmap(fn, 0)

    s = time.perf_counter_ns()
    run_mmap()
    e = time.perf_counter_ns()
    print((e - s) / 1e9)

    s = time.perf_counter_ns()
    threads = [threading.Thread(target=run_mmap) for _ in range(8)]
    for t in threads:
        t.start()
    for t in threads:
        t.join()
    e = time.perf_counter_ns()
    print((e - s) / 1e9)

On my laptop, the threaded portion goes from about 5-7s to 1-2s with this patch

bedevere-bot added the awaiting review label Oct 10, 2022

blurb-it bot and others added 2 commits Oct 10, 2022

📜🤖 Added by blurb_it.

1be0a04

Merge branch 'main' into pythongh-44098

95bb4a8

kumaraditya303 approved these changes Oct 10, 2022

View changes

bedevere-bot added awaiting core review and removed awaiting review labels Oct 10, 2022

kumaraditya303 requested a review from gvanrossum Oct 10, 2022

gvanrossum reviewed Oct 10, 2022

View changes

update news entry

7fb2b98

gvanrossum added the 🤖 automerge PR will be merged once it's been approved and all CI passed label Oct 10, 2022

gvanrossum approved these changes Oct 10, 2022

View changes

bedevere-bot added awaiting merge and removed awaiting core review labels Oct 10, 2022

gvanrossum changed the title ~~gh-44098: Release the GIL during mmap~~ gh-44098: Release the GIL during mmap on Unix Oct 10, 2022

miss-islington merged commit f871e9a into python:main Oct 10, 2022
15 checks passed

bedevere-bot removed the awaiting merge label Oct 10, 2022

hauntsaninja deleted the gh-44098 branch Oct 10, 2022

gh-44098: Release the GIL during mmap on Unix #98146

gh-44098: Release the GIL during mmap on Unix #98146

hauntsaninja commented Oct 10, 2022 •

edited by miss-islington

gvanrossum Oct 10, 2022

hauntsaninja Oct 10, 2022

miss-islington commented Oct 10, 2022

vstinner commented Oct 10, 2022

hauntsaninja commented Oct 10, 2022 •

edited

		@@ -0,0 +1 @@
		Release the GIL when creating :class:`mmap.mmap` objects.

gh-44098: Release the GIL during mmap on Unix #98146

gh-44098: Release the GIL during mmap on Unix #98146

Conversation

hauntsaninja commented Oct 10, 2022 • edited by miss-islington

gvanrossum Oct 10, 2022

Choose a reason for hiding this comment

hauntsaninja Oct 10, 2022

Choose a reason for hiding this comment

miss-islington commented Oct 10, 2022

vstinner commented Oct 10, 2022

hauntsaninja commented Oct 10, 2022 • edited

hauntsaninja commented Oct 10, 2022 •

edited by miss-islington

hauntsaninja commented Oct 10, 2022 •

edited