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

Support for M1 Macs? #35

Closed
torokati44 opened this issue Nov 1, 2022 · 22 comments
Closed

Support for M1 Macs? #35

torokati44 opened this issue Nov 1, 2022 · 22 comments

Comments

@torokati44
Copy link

torokati44 commented Nov 1, 2022

When trying to use this module on macOS on an ARM machine, this test fails on the following line:

self.assertEqual(mem.size, new_size)

With this message: AssertionError:16384 != 4096

This is more than likely due to these processors using a 16k page size rather than the more usual 4k. The Asahi Linux community has already written much about this.

@torokati44
Copy link
Author

I think there might be a confusion here between FS block size and memory page size.
os.statvfs returns the file system block size in its first value (f_bsize). Shared memory, not being on any real file system on any disk, apparently has not much to do with that.

@torokati44
Copy link
Author

torokati44 commented Nov 2, 2022

Not to mention the difference between the physical page size and the page size of the process... :|
See: https://docs.python.org/3/library/resource.html#resource.getpagesize

Either way, if the exact new value of mem.size is not that important, only that it's greater than zero (and maybe that it's a multiple of 1024 or 4096), perhaps the test could be made more lenient.

@osvenskan
Copy link
Owner

Thanks for the bug report! I have an M1 Mac, so I'll be able to experiment with this myself.

@osvenskan
Copy link
Owner

Interestingly, I'm unable to re-create this failure on my system (2021 M1 Pro, OS version 12.6). Can you tell me more about the system where you see this fail?

Also, can you copy/paste the output of this command?

python -c "import os; print(os.statvfs('.'))"

@rhornig
Copy link
Contributor

rhornig commented Nov 11, 2022

Actually we are trying to set up the given package with the NIX package manager on an M1 mac and the build f this package fails when it runs the tests. I will try to provide some additional data to reproduce it.

@tabgab
Copy link

tabgab commented Nov 11, 2022

os.statvfs_result(f_bsize=1048576, f_frsize=4096, f_blocks=242837545, f_bfree=142983978, f_bavail=142983978, f_files=1425860975, f_ffree=1424391824, f_favail=1424391824, f_flag=0, f_namemax=255)

@osvenskan
Copy link
Owner

I have two observations to make. First, the Open Group Specification (https://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html) says clearly --

If fildes refers to a shared memory object, ftruncate() shall set the size of the shared memory object to length.

Therefore, if macOS sets the size of the shared memory segment to anything other than what the caller passes, it's violating the specification. Apparently that's happening, otherwise this test wouldn't fail.

Second, I'm not sure how I settled on os.statvfs('.')[1] as the best way to get the file system block size. That's the f_frsize element of the structure returned by statvfs().

I have occasionally mined Python's source code for ideas on how to make code cross platform, and it wouldn't surprise me if I copied the statvfs() call from Python. I added that call in 2014 (46382e0) and in the commit notes I referred to it as "block size magic" which is evidence that I didn't understand it and copied it from elsewhere. Python 2.7 was the prominent Python at the time, and it uses f_frsize as the block size: https://github.com/python/cpython/blob/f4c03484da59049eb62a9bf7777b963e2267d187/Lib/multiprocessing/heap.py#L95

I probably copied it from there.

The documentation varies for f_bsize and f_frsize (both members of the struct returned by statvfs().

OpenGroup

unsigned long f_bsize    File system block size. 
unsigned long f_frsize   Fundamental file system block size. 
fsblkcnt_t    f_blocks   Total number of blocks on file system in units of f_frsize. 

https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/

IBM

unsigned long f_bsize 	The file system block size.
unsigned long f_frsize 	The fundamental file system block size.
unsigned long f_blocks 	The total number of blocks on the file system in units of f_frsize.

https://www.ibm.com/docs/en/zos/2.3.0?topic=functions-statvfs-get-file-system-information

Linux

unsigned long  f_bsize;    /* Filesystem block size */
unsigned long  f_frsize;   /* Fragment size */
fsblkcnt_t     f_blocks;   /* Size of fs in f_frsize units */

https://www.man7.org/linux/man-pages/man3/statvfs.3.html

macOS

This doc is for iPhoneOS; I had a hard time finding anything else.

f_bsize    The preferred length of I/O requests for files on this
           file system.  (Corresponds to the f_iosize member of
           struct statfs.)

f_frsize   The size in bytes of the minimum unit of allocation on
           this file system.  (This corresponds to the f_bsize member
           of struct statfs.)

About f_blocks, the documentation says "...the members f_blocks, f_bavail, and f_bfree (all of type fsblkcnt_t) represent the respective allocation-block counts."

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/fstatvfs.3.html

Summary About statvfs()

OpenGroup, IBM, and Linux seem to agree that f_frsize is the fundamental block size in this struct. Linux calls it "fragment size" for some odd reason, but it nevertheless defines f_blocks in units of f_frsize which is in agreement with OpenGroup and IBM's doc.

Apple is the odd one out here, probably due to its BSD roots. Nevertheless, it should be able to allocate a block of f_frsize.

Conclusion

The test is doing the right thing by relying on f_frsize. The behavior of macOS in this case disregards the spec. I'll add a special case for macOS to allow the shared memory's size to be ≥ the requested size.

@osvenskan
Copy link
Owner

I also want to point out for anyone reading that despite the title of the ticket, posix_ipc works just fine on M1 macs except for one test failing due to macOS allocating a larger memory segment than the caller requested.

@osvenskan
Copy link
Owner

@torokati44 @rhornig @tabgab the develop branch contains a fixed version of the test if you want to try it out.

@torokati44
Copy link
Author

Thank you so much, @osvenskan!
Would you be able to push a new release including this fix on PyPI in the near future?
It would make our work a lot easier.

@osvenskan
Copy link
Owner

Yes, I have a new release ready to go, but I was hoping one of you could test it out first since I can't re-create the problem on my Mac. Have you had a chance to test it out? Here's the change that should fix the failing test.

c505aea

@torokati44
Copy link
Author

Yes, I think @rhornig will try it out sometime soon.

@osvenskan
Copy link
Owner

posix_ipc 1.1.0 has been released; it contains the fix for this issue. If I don't hear back from anyone I'll assume this issue is resolved and close this soon.

https://pypi.org/project/posix-ipc/1.1.0/

@rhornig
Copy link
Contributor

rhornig commented Nov 29, 2022

My apologies for the late response. Sadly the change in c505aea did not fix all test failures (only 1 out of 3). I was able to set up the package on my machine and give a proper fix in the above pull request #41. The main issue was that mmap rounds the returned size to the pagesize of the OS and not the filesystem block size. This was the same on Linux and intel based MACs, but is different on aarch64 macOS machines (pagesize is 16384 here)

@rhornig
Copy link
Contributor

rhornig commented Nov 29, 2022

A few refernces:

@rhornig
Copy link
Contributor

rhornig commented Dec 8, 2022

Make sure that you run the tests in arm64 (native) mode. In i386 mode, the operating system will report back a page size of 4096 and all tests will pass.

@osvenskan
Copy link
Owner

@rhornig Thanks for the update. I finally got a chance to update my Python to arm64 native and I can re-create the problem. I have no doubt your PR fixes it, but I need to figure out how to make sure that change doesn't break on other platforms. Some I can test under AWS EC2 instances, but I lost the ability to test FreeBSD when I moved to an M1 Mac since VirtualBox can't emulate x86 on ARM (yet).

@osvenskan
Copy link
Owner

Notes to myself: I ran the snippet below on several Linux ARM & x86 platforms, an M1 Mac (using ARM-compiled Python), and an x86 Mac (courtesy of a friend) --

python3 -c "import os; import mmap; bs = os.statvfs('.')[1]; print(f'block: {bs}'); print(f'page: {mmap.PAGESIZE}')"

On all platforms except the M1 Mac, statvfs() and mmap.PAGESIZE produces the same output --

block: 4096
page: 4096

Only on M1 Mac w/ARM Python are the values different --

block: 4096
page: 16384

That suggests #41 is a good fix. I haven't been able to test xBSD yet, nor am I likely to be able to. :-/ I would also like to test x86 Python on an M1 Mac just to be on the safe side. It's not hard to do, I just need to find the time. Based on previous experience I think it will be the 4096/4096 answer.

@osvenskan
Copy link
Owner

Thanks to a tip from @bpollack I learned that vultr.com has FreeBSD VMs, and I'm happy to report that the test above reports 4096 for both values.

$ uname -a
FreeBSD bsdtest 13.1-RELEASE-p3 FreeBSD 13.1-RELEASE-p3 GENERIC amd64
$ python3 -c "import os; import mmap; bs = os.statvfs('.')[1]; print(f'block: {bs}'); print(f'page: {mmap.PAGESIZE}')"
block: 4096
page: 4096

@osvenskan
Copy link
Owner

Confirmed that x86 Python on M1 Mac returns 4096/4096 using the command above.

osvenskan added a commit that referenced this issue Dec 31, 2022
fix for #35: Failing memory test on aarch64 based macOS.
@osvenskan
Copy link
Owner

Reopening until I release a new version that contains the fix.

@osvenskan osvenskan reopened this Dec 31, 2022
@osvenskan
Copy link
Owner

This is fixed in version 1.1.1 which is now released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants