-
Notifications
You must be signed in to change notification settings - Fork 20
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
Total and free size of filesystem reported in sites doesn't match actual values #341
Comments
I explored the Playground code and noticed that the free and total values are fixed values. Let me briefly explain the references I found while debugging this: Free space
Total space
Filesystem values
UPDATE: As shared in #341 (comment), the values are set by Emscripten here. |
CC @adamziel |
@fluiddot I went a bit deeper and it looks like these values are set in emscripten: https://github.com/emscripten-core/emscripten/blob/7f8a05dd4e37cbd7ffde6d624f91fd545f7b52e3/src/library_syscall.js#L804 With the various types of available VFS's it could be complicated to give a meaningful output of statfs which is probably why they settled on a stub with, as they call it, sane values. @adamziel Do you have any ideas of how we can proceed with this? I'm happy to have a deeper look but I'm out of my depth here so some pointers would be very welcome :-) |
It seems like it's reporting the heap size, not the disk size. The 2GB comes up on this Emscripten doc page but not directly tied with this problem. The original commit introducing these constants in Emscripten says:
It sounds like this could be solved with an Emscripten PR that delegates the |
This approach could work. Some filesystems might not be relevant because they only work in the browsers. For NODEFS this will work and I was able to test that with some custom code: function ___syscall_statfs64(path, size, buf) {
let defaults = {
bsize: 4096,
frsize: 4096,
blocks: 1e6,
bfree: 5e5,
bavail: 5e5,
files: FS.nextInode,
ffree: 1e6,
fsid: 42,
flags: 2,
namelen: 255
}
try {
path = SYSCALLS.getStr(path);
if (ENVIRONMENT_IS_NODE ) { // need to check here if FS is nodefs too.
const node = FS.lookupPath(SYSCALLS.getStr(path), { follow: true }).node;
const stats = fs.statfsSync(node.mount.opts.root);
defaults.bsize = stats.bsize;
defaults.frsize = stats.bsize;
defaults.blocks = stats.blocks;
defaults.bfree = stats.bfree;
defaults.bavail = stats.bavail;
defaults.files = stats.files;
defaults.ffree = stats.ffree;
defaults.fsid = stats.type;
}
HEAP32[buf + 4 >> 2] = defaults.bsize;
HEAP32[buf + 40 >> 2] = defaults.frsize;
HEAP32[buf + 8 >> 2] = defaults.blocks;
HEAP32[buf + 12 >> 2] = defaults.bfree;
HEAP32[buf + 16 >> 2] = defaults.bavail;
HEAP32[buf + 20 >> 2] = defaults.files;
HEAP32[buf + 24 >> 2] = defaults.ffree;
HEAP32[buf + 28 >> 2] = defaults.fsid;
HEAP32[buf + 44 >> 2] = defaults.flags;
HEAP32[buf + 36 >> 2] = defaults.namelen;
return 0;
} catch (e) {
if (typeof FS == "undefined" || !(e.name === "ErrnoError"))
throw e;
return -e.errno;
}
} As far as i can see the values are correctly added to the shared memory and read by the systemcall inside WASM. I will create an issue on emscripten. There's probably a bit more that we need to consider and each filesystem might need different defaults or ways to determine more accurate numbers. |
I created an issue here: emscripten-core/emscripten#22607 - if this approach has any merit, we could work on it. At least for NODEFS it seems to be fairly easy to add support for it. |
@fluiddot @wojtekn I created a first concept of how we could solve it. Things like error handling are still missing. The idea is to use the FS instance the same way it is used throughout the code. It will try to call the Here is a branch that i am working in: emscripten-core/emscripten@main...jeroenpf:emscripten:fix-22607 - once it is closer to being finished I'll create a PR. To run this I created a small test program:
#include <stdio.h>
#include <sys/vfs.h>
#include <errno.h>
#include <string.h>
#include <dirent.h>
#include <emscripten.h>
#include <inttypes.h> // For PRIu32/64 macros
void print_statfs_info(const char* path) {
struct statfs s;
if (statfs(path, &s) != 0) {
printf("statfs failed for %s: %s\n", path, strerror(errno));
return;
}
printf("Filesystem information for %s:\n", path);
printf("Block size: %lu\n", s.f_bsize);
printf("Fragment size: %lu\n", s.f_frsize);
printf("Total blocks: %" PRIu32 "\n", s.f_blocks);
printf("Free blocks: %" PRIu32 "\n", s.f_bfree);
printf("Available blocks: %" PRIu32 "\n", s.f_bavail);
printf("Total inodes: %" PRIu32 "\n", s.f_files);
printf("Free inodes: %" PRIu32 "\n", s.f_ffree);
}
int main() {
#ifndef NODERAWFS
// Run some js that mounts the current directory at /working mounpoint
EM_ASM(
FS.mkdir('/working');
FS.mount(NODEFS, { root: '.' }, '/working');
);
#endif
// Verify that the directory is mounted
DIR *dir;
dir = opendir("/working");
if (dir == NULL) {
printf("Failed to open directory: %s\n", strerror(errno));
return 1;
}
// Print the statfs information for the /working directory
print_statfs_info("/working/README.md");
return 0;
} To compile wasm: To run: |
Was this resolved? |
@adamziel no, we need to compile PHP-WASM in the Playground repository and then upgrade it in Studio. |
@brandonpayton is working on updating Emscripten to 3.1.74 as part of the PHP 8.4 work. |
Here is an in-progress PR for that upgrade: I am AFK until the end of the week but plan to continue the PR afterward unless someone else finishes it before then. |
Quick summary
The total and free size of filesystem reported in sites doesn't match the ones from the machine. This can lead to issues in plugins that rely on these values.
Steps to reproduce
wp-config-php
file of the site and add the following code at the bottom:<SITE_PATH>/wp-content/debug.log
.What you expected to happen
The size of filesystem matches the values of the machine.
What actually happened
The size of the filesystem doesn't match the values of the machine.
Impact
All
Available workarounds?
No but the app is still usable
Platform
Mac Silicon
Logs or notes
No response
The text was updated successfully, but these errors were encountered: