vgc: improve, big step forward#27855
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6279ff0ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if npages >= 32 { | ||
| // Large span: keep on the dedicated list, matched by exact page count. | ||
| unsafe { | ||
| span.next = vgc_heap.free_large | ||
| vgc_heap.free_large = span |
There was a problem hiding this comment.
On Windows, vgc_os_decommit() is implemented with VirtualFree(..., MEM_DECOMMIT), so adding large spans to free_large after this call makes vgc_get_free_span() later return an uncommitted address for the next exact-sized large allocation. That path immediately writes to the span (for example via memset in vgc_alloc_large()), which will fault on Windows for large objects that survive to a sweep and are then reused; POSIX MADV_DONTNEED does not have the same requirement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'll take a short break for now to check the patch and the new fix for Windows on Windows 11.
I got curious to try out how it would crash :)
2 participants
Fixes #27830
Despite the fact that this patch contains many changes, it has to be done as a single chunk; this courageous decision has to be made :(.
In addition to fixing the bug that the user found, vgc will now start passing the tests from
vlib/(i am so happy), and in the future it will be easier to make commits when you know that you haven't broken anything.The patch contains previous findings and knowledge that were made as soon as
vgcappeared in April of this year.Highlights:
alltests for the first time -v -gc vgc test vlib/[a-c,m]*v run bench/bench_gc.vuses less memory thanboehm