Skip to content

runtime: add MemStats.NumGC for API compatibility - #5590

Merged
dgryski merged 1 commit into
tinygo-org:devfrom
0magnet:runtime-numgc
Aug 18, 2026
Merged

runtime: add MemStats.NumGC for API compatibility#5590
dgryski merged 1 commit into
tinygo-org:devfrom
0magnet:runtime-numgc

Conversation

@0pcom

@0pcom 0pcom commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

runtime.MemStats is missing NumGC, so any program that reads it fails to compile — including programs that only log it.

TinyGo's GC does not count cycles, so the field stays 0 and the doc comment says exactly that rather than implying a number nobody maintains. The same approach the struct already takes for other fields it cannot fill.

Verified by building TinyGo.

@b0ch3nski b0ch3nski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dgryski

dgryski commented Aug 14, 2026

Copy link
Copy Markdown
Member

Normally when we add fields to runtime.MemStats, we try to implement them for the garbage collectors where possible. Could you add a gcNumGC int to gc_blocks.go (here https://github.com/tinygo-org/tinygo/blob/dev/src/runtime/gc_blocks.go#L59 ) and track the number of garbage collections? For boehmgc, it's the gc_no field of the returned prof_stats struct.

@0pcom

0pcom commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Good call — it is now actually tracked rather than a constant zero.

  • gc_blocks: added gcNumGC and increment it at the end of runGC rather than in GC(), so collections triggered by an allocation are counted too, not just explicit runtime.GC() calls. Every runGC caller already holds gcLock, which is the same lock ReadMemStats reads the other counters under.
  • gc_boehm: reports bdwgc's own gc_no from the prof_stats struct, as you suggested.
  • gc_leaking: stays 0, and now says why — that collector never completes a cycle.

The doc comment on the field no longer claims TinyGo does not track this. Rebased on dev.

@dgryski dgryski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@dgryski

dgryski commented Aug 17, 2026

Copy link
Copy Markdown
Member

Can you please fix the sizes test so we can get a clean CI run?

Reading runtime.MemStats.NumGC is common enough in dependency code that its
absence is a compile error for programs that never look at the value. Rather
than add the field as a constant zero, track it:

- gc_blocks: count completed cycles in runGC, so collections triggered by an
  allocation are counted as well as explicit runtime.GC() calls. The counter is
  read and written under gcLock, like the other counters beside it.
- gc_boehm: report bdwgc's own gc_no from the prof_stats struct.
- gc_leaking: always 0, since that collector never completes a cycle.
@0pcom

0pcom commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Done — the three TestBinarySize expectations are updated to what CI measured.

Since that test asks you to weigh whether an increase is worth it, here is the cost rather than just the new numbers:

target code bss
hifive1b/echo 4313 → 4321 (+8) 2260 → 2268 (+8)
microbit/serial 2838 → 2842 (+4) 2256 → 2264 (+8)
wioterminal/pininterrupt 8027 → 8039 (+12) 7488 → 7496 (+8)

So roughly 8 bytes of BSS and 4–12 of code on a microcontroller. The BSS figure is 8 rather than 4 because the uint32 counter picks up alignment padding next to the existing globals.

Worth being explicit that this is paid by every user of gc_blocks, including those who never call ReadMemStats — the counter is written in runGC and only read there, and evidently is not being optimised out. If that trade is not one you want, the alternative is to drop the counter and report NumGC only where it is free: bdwgc already tracks gc_no, and the leaking collector is honestly 0. That would leave gc_blocks reporting 0, which is what the original version of this PR did and what you objected to — so I would rather you make the call than guess again.

Happy either way; say which and I will push it.

@dgryski dgryski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@dgryski
dgryski merged commit 338af91 into tinygo-org:dev Aug 18, 2026
18 of 20 checks passed
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.

3 participants