Skip to content

Conversation

kleisauke
Copy link
Collaborator

No description provided.

```
../../../system/lib/mimalloc/src/os.c:36:3: error: implicit conversion from 'unsigned long long' to 'size_t' (aka 'unsigned long') changes value from 4294967296 to 0 [-Werror,-Wconstant-conversion]
   32 | static mi_os_mem_config_t mi_os_mem_config = {
      |                                              ~
   33 |   4096,     // page size
   34 |   0,        // large page size (usually 2MiB)
   35 |   4096,     // allocation granularity
   36 |   MI_DEFAULT_PHYSICAL_MEMORY,
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~
../../../system/lib/mimalloc/src/os.c:26:40: note: expanded from macro 'MI_DEFAULT_PHYSICAL_MEMORY'
   26 | #define MI_DEFAULT_PHYSICAL_MEMORY    4*MI_GiB
      |                                       ~^~~~~~~
1 error generated.
```
@kleisauke
Copy link
Collaborator Author

Context: this PR seems to address a suspicious(?) OOM error on wasm-vips' test suite. See for example the CI results for commit kleisauke/wasm-vips@605b53f and kleisauke/wasm-vips@5cb048b.
https://github.com/kleisauke/wasm-vips/actions/runs/12053482829/job/33610497071#step:7:302
https://github.com/kleisauke/wasm-vips/actions/runs/12053529699

It's generally recommended to use tagged upstream versions, but I prefer to avoid those that are known to have issues. Note that the previous version of mimalloc (used prior to PR #21548) was commit microsoft/mimalloc@4e50d67, which is also an untagged version.

#define MIMALLOC_H

#define MI_MALLOC_VERSION 217 // major + 2 digits minor
#define MI_MALLOC_VERSION 188 // major + 2 digits minor
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I just noticed that I synced with the dev branch. Perhaps it should be synced with the dev-slice branch instead.

### Branches
* `master`: latest stable release (based on `dev-slice`).
* `dev`: development branch for mimalloc v1. Use this branch for submitting PR's.
* `dev-slice`: development branch for mimalloc v2. This branch is downstream of `dev` (and is essentially equal to `dev` except for
`src/segment.c`)

@kleisauke
Copy link
Collaborator Author

Updating to the latest commit on the dev-slice branch did not resolve the issue (see kleisauke/wasm-vips@28b407a and HEAD...kleisauke:mimalloc-update-577246d for details).

Upon further investigation, it appears to be the same issue as outlined in #20645 (comment), i.e. mimalloc requests an alignment of 4 MiB, causing emmalloc to allocate 8 MiB (or perhaps was that fixed via 3cd6861?), which somehow exceeds the -sINITIAL_MEMORY=1GB limit in wasm-vips' test suite. Halving the page size seems to fix this.

Closing in favor of #23037.

@kleisauke kleisauke closed this Nov 29, 2024
@kleisauke kleisauke deleted the mimalloc-update-9b75377 branch November 29, 2024 11:15
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

Successfully merging this pull request may close these issues.

1 participant