[font/opentype] add ResourceSlicer, so tables are not copied by mike-ward · Pull Request #299 · go-text/typesetting

mike-ward · 2026-10-06T16:02:52Z

Loader.RawTable copies every table out of the Resource into a new buffer, and a font.Font keeps the tables it parses. For a color emoji font that is most of the file. The sbix table of macOS Apple Color Emoji.ttc is 182 MB, so NewFont on it keeps 193 MB on the Go heap. A terminal that falls back to this font for one emoji grows by that much.

As suggested in review, this PR adds an optional interface, checked in findTableBuffer:

type ResourceSlicer interface {
	Resource
	Slice(start, end int64) ([]byte, error)
}

func NewSliceResource(data []byte) ResourceSlicer

When the Resource given to NewLoader or NewLoaders is a ResourceSlicer, each uncompressed table is the slice that Slice returns. It is not copied. NewSliceResource wraps a bytes.Reader for callers that have the whole file in memory. The data can also be a read-only memory mapping of the file. The OS then pages the tables in as they are used, and can drop the pages again under memory pressure.

The capacity of each returned table stops at the table end, so if a caller appends to it, the next table is not overwritten.
If Slice returns an error, or a slice of the wrong length, RawTable returns an error.
Compressed WOFF tables are still decoded into a new buffer.
On this path dst in RawTableTo is not used. dst can be a table that an earlier call returned (NewFont and fontscan pass the previous buffer back), and that table is now a view of read-only memory.
NewSliceResource rejects the same table ranges as bytes.Reader.ReadAt, so both paths accept the same fonts.
A Resource that does not implement Slice works as before.

I checked that nothing in font or tables writes into a table buffer. The only write found, in KernData1.parseValues, goes to a [4]byte array field.

Measured

Apple Color Emoji.ttc, face 0, heap after runtime.GC() with the Font alive:

Resource	Heap
`os.File`	+193 MB
`syscall.Mmap` + `NewSliceResource`	+11 MB

U+1F600 still decodes as a PNG GlyphBitmap.

Tests

Every table of each common and collections font (this includes a compressed WOFF) reads back the same as from a bytes.Reader.
Each uncompressed table is a view of the input, with cap == len.
RawTableTo with the previous table as dst leaves the input unchanged.
A truncated input returns errors and does not panic.
A Slice error, or a short slice, from an outside implementation is returned as an error.
In font, sbix glyph data from the Sbix toy fonts is the same as from a bytes.Reader, and it points into the input.

go test ./..., go vet and staticcheck v0.8.2 are clean.

🤖 Generated with Claude Code

RawTable copies every table out of the Resource into a new buffer, and a font.Font keeps the tables it parses. For a color emoji font this is most of the file: the 'sbix' table of macOS Apple Color Emoji.ttc is 182MB, so NewFont on it keeps 193MB on the Go heap. NewLoadersFromBytes builds the loaders from a []byte and returns each uncompressed table as a sub-slice of it, with its capacity limited to the table end. Compressed WOFF tables are still decoded into a new buffer. The input can be a read-only memory mapping, so the OS pages the tables in as they are used and can drop them again under memory pressure. The dst argument of RawTableTo is not used by such a loader: it may be a table returned before, which is then a view of the input. Measured on Apple Color Emoji.ttc, face 0, heap after GC with the Font alive: os.File + NewLoaders +193MB; mmap + NewLoadersFromBytes +11MB. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

TestParseCrashers already feeds random input to NewLoaders, which NewLoadersFromBytes calls. The loop also used math/rand.Read, which staticcheck flags (SA1019). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…table at EOF - Doc: the whole input stays reachable while any loader, font or data derived from them is in use; it must not be modified or unmapped; a returned table must never be passed as dst to a copying loader. - An empty table at offset == len(data) is now rejected, as ReadAt does on the copying path. - Tests: a compressed WOFF table with a view of the input as dst (fails without the dst guard); truncated input must accept and reject the same tables as NewLoaders; empty table at EOF. The old dst test is removed: it passed without the guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

benoitkugler · 2026-10-08T14:08:36Z

Hello,

While I sympathize with the issue, I'm not sure I like the proposed solution that much. It seems the issue is that findTableBuffer always allocate (when dst is nil). It is indeed unfortunate when using read-only memory mapping.

Could we instead introduce an interface extending Resource with the following method :
Slice(start, end int) []byte
and change findTableBuffer to rely on such a method if present ?

Then, the new interface is easy to implement if you already have the whole resource as a byte slice.

mike-ward · 2026-10-08T16:47:19Z

Thanks, that is cleaner. I'll rework it as an optional interface checked in findTableBuffer. Two questions:

Can Slice return an error, Slice(start, end int64) ([]byte, error)? Then a bad table offset returns an error and does not panic, and the types match ReadAt.
Do you want an exported type that wraps []byte and has Slice, or should callers write their own?

On the slice path, dst will still be ignored (a previous result can be a view of read-only memory), and the returned slice will be capped at the table end.

benoitkugler · 2026-10-08T19:48:40Z

Thanks, that is cleaner. I'll rework it as an optional interface checked in findTableBuffer. Two questions:

Can Slice return an error, Slice(start, end int64) ([]byte, error)? Then a bad table offset returns an error and does not panic, and the types match ReadAt.

Do you want an exported type that wraps []byte and has Slice, or should callers write their own?

On the slice path, dst will still be ignored (a previous result can be a view of read-only memory), and the returned slice will be capped at the table end.

Yes, returning an error should be fine.

I'm not sure about exporting a concrete type wrapping a slice. Maybe that would benefit other users who already have the full slice "in memory" ?

mike-ward · 2026-10-08T22:06:38Z

Thanks. I'll add ResourceSlicer (Resource + Slice(start, end int64) ([]byte, error)), checked in findTableBuffer, and drop NewLoadersFromBytes. I'll also export a small NewSliceResource([]byte) wrapper over bytes.Reader, since any mmap or in-memory user needs one. It is easy to remove if you'd rather not have it.

As suggested in review, the zero-copy path is now an optional interface checked in findTableBuffer, and not a separate constructor: - ResourceSlicer is a Resource with Slice(start, end int64) ([]byte, error). When the Resource given to NewLoader(s) implements it, uncompressed tables are returned as the slice from Slice, capped at the table end. dst is not used, since a previous result can be a view of read-only memory. - NewSliceResource([]byte) wraps a bytes.Reader for callers that already have the whole file in memory, or as a memory mapping. - A Slice error, or a slice of the wrong length, is returned as an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mike-ward · 2026-10-13T13:20:04Z

@benoitkugler The rework is pushed (b157558):

ResourceSlicer (Resource + Slice(start, end int64) ([]byte, error)) is checked in findTableBuffer. NewLoadersFromBytes is removed.
NewSliceResource([]byte) wraps a bytes.Reader. It is easy to remove if you'd rather callers write their own.
On the slice path, dst is not used, and each table is capped at its end. A Slice error, or a slice of the wrong length, is returned as an error.

The heap result is the same as before: +11 MB with an mmap of Apple Color Emoji, against +193 MB with an os.File. The PR description has the details.

mike-ward · 2026-10-13T13:24:35Z

The static_analysis failure is not caused by this PR. analysis.yml installs staticcheck@v0.8.0 with go-version: 'stable', which is now Go 1.27. Staticcheck v0.8.0 cannot read the export data of Go 1.27:

internal error in importing "internal/byteorder" (cannot decode "internal/byteorder", export data version 5 is greater than maximum supported version 4)

It stops before it checks any code. Other branches fail the same way, for example fontscan/system-fonts-without-cache on Oct 11. With staticcheck v0.8.2, this branch has no findings. All six test jobs pass.

Changing the pin in .github/workflows/analysis.yml to staticcheck@v0.8.2 fixes it. I can open a separate PR for that if you want.

benoitkugler

Thank you for this new version.
I have left two comments I would like to adress before merging, but the direction looks right overall !

benoitkugler · 2026-10-14T11:55:49Z

+			// compare as uint64: int(s.length) can overflow on 32-bit platforms
+			if uint64(len(table)) != uint64(s.length) {
+				return nil, fmt.Errorf("invalid table: Slice(%d, %d) returned %d bytes", start, end, len(table))
+			}


This looks odd to me : I would prefer to move the check (if required) to the ResourceSlicer implementation, and trust its result.

Done in 9ae34df. The Slice contract now requires exactly end - start bytes, and findTableBuffer returns its result without a check.

benoitkugler · 2026-10-14T11:56:01Z

+			// limit the capacity so that appending to the table can not
+			// overwrite the next one
+			return table[:len(table):len(table)], nil


That seems not required given the contract of ResourceSlicer :

The capacity of the returned table is limited to its length.

Done in 9ae34df. The capacity limit is now part of the Slice contract, sliceResource.Slice returns data[start:end:end], and the limit in findTableBuffer is removed.

The Slice contract now requires a result of exactly end - start bytes, with capacity limited to its length. sliceResource enforces it, and findTableBuffer returns the result without checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

benoitkugler

This looks good to me. I would love to have the approval of @andydotxyz or @whereswaldon though, since there is an API addition.

andydotxyz

Seems like a good change - the interface API etc is clean (if a little doc heavy) - but the constructor function name doesn't seem right.

andydotxyz · 2026-10-14T20:15:43Z

+// the operating system loads the tables in memory when they are used.
+//
+// See [ResourceSlicer] for how [data] is used.
+func NewSliceResource(data []byte) ResourceSlicer {


This method name does not read well in my mind. Should it be NewSlicedResource - or NewSliceableResource perhaps? open to other suggestions too

Are you asking the other contributors or me?

Anyone who can help move it forward I guess

The name reads fine to me as is. That being said, I'm not married to it. Pick a new one, and I'll change it.

mike-ward requested review from andydotxyz, benoitkugler and whereswaldon as code owners October 6, 2026 16:02

mike-ward and others added 2 commits October 6, 2026 11:07

[font/opentype] drop random-input loop from NewLoadersFromBytes test

fe13ac5

TestParseCrashers already feeds random input to NewLoaders, which NewLoadersFromBytes calls. The loop also used math/rand.Read, which staticcheck flags (SA1019). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mike-ward changed the title ~~[font/opentype] add NewLoadersFromBytes, which does not copy tables~~ [font/opentype] add ResourceSlicer, so tables are not copied Oct 13, 2026

benoitkugler requested changes Oct 14, 2026

View reviewed changes

benoitkugler approved these changes Oct 14, 2026

View reviewed changes

andydotxyz requested changes Oct 14, 2026

View reviewed changes

Conversation

mike-ward commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Measured

Tests

Uh oh!

benoitkugler commented Oct 8, 2026

Uh oh!

mike-ward commented Oct 8, 2026

Uh oh!

benoitkugler commented Oct 8, 2026

Uh oh!

mike-ward commented Oct 8, 2026

Uh oh!

mike-ward commented Oct 13, 2026

Uh oh!

mike-ward commented Oct 13, 2026

Uh oh!

benoitkugler left a comment

Choose a reason for hiding this comment

Uh oh!

benoitkugler Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mike-ward Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

benoitkugler Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mike-ward Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

benoitkugler left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andydotxyz left a comment

Choose a reason for hiding this comment

Uh oh!

andydotxyz Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mike-ward Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

andydotxyz Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

mike-ward Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mike-ward commented Oct 6, 2026 •

edited

Loading

benoitkugler Oct 14, 2026 •

edited

Loading

benoitkugler left a comment •

edited

Loading