Skip to content

all: shrink bdwgc library #4832

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion builder/bdwgc.go
Original file line number Diff line number Diff line change
@@ -30,6 +30,10 @@ var BoehmGC = Library{
// Use a minimal environment.
"-DNO_MSGBOX_ON_ERROR", // don't call MessageBoxA on Windows
"-DDONT_USE_ATEXIT",
"-DNO_CLOCK", // don't use system clock
"-DNO_DEBUGGING", // reduce code size
"-DGC_NO_FINALIZATION", // finalization is not used at the moment
"-DNO_GETENV", // smaller binary, more predictable configuration
Copy link

Choose a reason for hiding this comment

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

"-DGC_DISABLE_INCREMENTAL" should also help in decreasing code size

Copy link

Choose a reason for hiding this comment

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

Also, consider -D SMALL_CONFIG (which implies GC_DISABLE_INCREMENTAL) and -D IGNORE_DYNAMIC_LOADING.

Copy link
Member Author

@aykevl aykevl Mar 29, 2025

Choose a reason for hiding this comment

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

I already use -DGC_DISABLE_INCREMENTAL and -DIGNORE_DYNAMIC_LOADING, see a few lines above.
(I might try to enable incremental GC at some point, but I'll need to be careful around handling SIGSEGV etc so it's disabled at the moment. Even with incremental GC disabled, bdwgc is still a lot faster than our own naive GC).

-D SMALL_CONFIG docs say:

Tries to tune the collector for small heap sizes, usually causing it to use less space in such situations. Incremental collection no longer works in this case.

Some people might want to use a large heap size, I'd like to keep that ability. This is a generic configuration, not for a specific program so would like to avoid tuning for a specific configuration.

Copy link

Choose a reason for hiding this comment

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

You could define -D LARGE_CONFIG in conjunction with -D SMALL_CONFIG. It might seem confllusing a bit:)

Copy link

Choose a reason for hiding this comment

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

Another available option (with the latest bdwgc master): -D NO_FIND_LEAK


// Special flag to work around the lack of __data_start in ld.lld.
// TODO: try to fix this in LLVM/lld directly so we don't have to
@@ -38,6 +42,8 @@ var BoehmGC = Library{

// Do not scan the stack. We have our own mechanism to do this.
"-DSTACK_NOT_SCANNED",
"-DNO_PROC_STAT", // we scan the stack manually (don't read /proc/self/stat on Linux)
"-DSTACKBOTTOM=0", // dummy value, we scan the stack manually

// Assertions can be enabled while debugging GC issues.
//"-DGC_ASSERTIONS",
@@ -59,7 +65,6 @@ var BoehmGC = Library{
"blacklst.c",
"dbg_mlc.c",
Copy link

Choose a reason for hiding this comment

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

With the latest bdwgc master: dbg_mlc.c is not needed unless you use GC_debug_* API.

"dyn_load.c",
"finalize.c",
Copy link

Choose a reason for hiding this comment

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

I think there's no need to exclude finalize.c as it should compile to zero size if GC_NO_FINALIZATION is defined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, but there's no reason to include it either 🤷

"headers.c",
"mach_dep.c",
"malloc.c",