GODRIVER-3868 Update extended bson cases. by qingyang-hu · Pull Request #2400 · mongodb/mongo-go-driver

qingyang-hu · 2026-05-26T15:03:04Z

Summary

Update the benchmark framework to run directly against the specifications sub-repo.
This PR decompresses the tgz file and unmarshals the embedded JSON files.

Background & Motivation

We no longer need to maintain local copies for test cases because we use a sub-repo for the test data.

mongodb-drivers-pr-bot · 2026-05-26T15:08:48Z

🧪 Performance Results

Commit SHA: b17879b

The following benchmark tests for version 6a1eda7b30aa0f00077a65e6 had statistically significant changes (i.e., |z-score| > 1.96):

Benchmark	Measurement	% Change	Patch Value	Stable Region	H-Score	Z-Score
BenchmarkBSONDeepDocumentDecoding	ns_per_op	-3.3181	61217.0000	Avg: 63317.9769 Med: 63343.5000 Stdev: 1059.5067	0.7247	-1.9830
BenchmarkBSONDeepDocumentDecoding	ops_per_second_med	3.2634	17254.4689	Avg: 16709.1851 Med: 16683.9069 Stdev: 264.6535	0.7327	2.0604
BenchmarkBSONDeepDocumentDecoding	ops_per_second_max	2.8735	17768.9328	Avg: 17272.6051 Med: 17200.4541 Stdev: 253.0210	0.7288	1.9616
BenchmarkBSONDeepDocumentEncoding	total_time_seconds	-1.7487	1.1732	Avg: 1.1941 Med: 1.1945 Stdev: 0.0092	0.7798	-2.2618
BenchmarkBSONFlatDocumentDecoding	total_time_seconds	-1.4589	1.1820	Avg: 1.1995 Med: 1.1992 Stdev: 0.0063	0.8095	-2.7585

For a comprehensive view of all microbenchmark results for this PR's commit, please check out the Evergreen perf task for this patch.

mongodb-drivers-pr-bot · 2026-05-26T15:27:51Z

API Change Report

No changes found!

Copilot

Pull request overview

Updates the driver’s benchmark fixtures to consume the Extended BSON benchmark data directly from the specifications git submodule tarball (extended_bson.tgz) instead of relying on locally maintained copies, aligning benchmark execution with upstream spec data.

Changes:

Added logic in the internal benchmark framework to locate and extract a specific Extended JSON fixture from extended_bson.tgz.
Updated bson package benchmarks to read fixtures from extended_bson.tgz, caching parsed entries for reuse.
Renamed benchmark case labels from *.json.gz to *.json to reflect the new fixture source format.

Reviewed changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated 2 comments.

File	Description
internal/cmd/benchmark/benchmark_test.go	Loads Extended BSON benchmark fixtures directly from the specifications tarball for internal benchmark runs.
bson/benchmark_test.go	Replaces per-file gz fixture reads with tarball extraction + cached unmarshalling for BSON package benchmarks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	tgzPath := filepath.Join(testdataDir(b), "specifications", "source", "benchmarking", "data", "extended_bson.tgz")
+
+	file, err := os.Open(tgzPath)
+	require.NoError(b, err, "failed to open %q", tgzPath)
+	defer file.Close()
+
+	gz, err := gzip.NewReader(file)
+	require.NoError(b, err, "failed to create gzip reader")
+	defer gz.Close()


+		tr := tar.NewReader(gz)
+		for {
+			hdr, err := tr.Next()
+			if errors.Is(err, io.EOF) {
+				break
+			}
+			if err != nil {
+				b.Fatalf("error reading tar: %s", err)
+				return nil


prestonvasquez · 2026-05-27T16:03:41Z

-	defer func() {
-		_ = gz.Close()
-	}()
+	if extJSONFiles == nil {


[nit] Suggest using sync.Once to help with readability:

func readExtJSONFile(b *testing.B, filename string) map[string]any { b.Helper() extJSONFilesOnce.Do(func() { // load into extJSONFiles }) v, ok := extJSONFiles["extended_bson/"+filename] if !ok { b.Fatalf("file %q not found in %q", filename, extendedBSONTGZ) return nil } return v }

prestonvasquez · 2026-05-27T16:04:59Z

+			return nil
+		}
+		defer func() {
+			_ = file.Close()


[blocking] We should check this error.

I don't think the closing error affects loading, and we don't need to log it either.

prestonvasquez · 2026-05-27T16:05:13Z

+			return nil
+		}
+		defer func() {
+			_ = gz.Close()


[blocking] We should check this error.

I don't think the closing error affects loading, and we don't need to log it either.

prestonvasquez · 2026-05-27T16:07:52Z

-	if err != nil {
-		panic(fmt.Sprintf("error reading GZIP contents of file: %s", err))
-	}
+		extJSONFiles = make(map[string]map[string]any)


[nit] It doesn't matter in this case but for hygiene suggest populating a local map and assigning this at the end of sync.Once, to avoid partially filling and then erroring.

prestonvasquez · 2026-05-27T16:08:30Z

+	if extJSONFiles == nil {
+		file, err := os.Open(extendedBSONTGZ)
+		if err != nil {
+			b.Fatalf("error opening %q: %s", extendedBSONTGZ, err)


Suggested change

b.Fatalf("error opening %q: %s", extendedBSONTGZ, err)

b.Fatalf("error opening %q: %v", extendedBSONTGZ, err)

prestonvasquez · 2026-05-27T16:09:03Z

+			}
+			data, err := io.ReadAll(tr)
+			if err != nil {
+				b.Fatalf("error reading tar entry %q: %s", hdr.Name, err)


Suggested change

b.Fatalf("error reading tar entry %q: %s", hdr.Name, err)

b.Fatalf("error reading tar entry %q: %v", hdr.Name, err)

prestonvasquez · 2026-05-27T16:09:12Z

+			}
+			var v map[string]any
+			if err = UnmarshalExtJSON(data, false, &v); err != nil {
+				b.Fatalf("error unmarshalling %q: %s", hdr.Name, err)


Suggested change

b.Fatalf("error unmarshalling %q: %s", hdr.Name, err)

b.Fatalf("error unmarshalling %q: %v", hdr.Name, err)

prestonvasquez · 2026-05-27T18:18:04Z

+// readExtJSONFile reads the named JSON file from the extended_bson.tgz archive and returns it as a
+// map[string]any. The first call decompresses the archive and caches all entries; subsequent calls
+// only look up the cache. It calls b.Fatal on any errors.
+func readExtJSONFile(b *testing.B, filename string) map[string]any {


[blocking] Use testing.TB instead of *testing.B so that the loader is agnostic. In addition, I think we should make a test for the loader to ensure the structure is what we expect.

We can use the hard-coded file names underneath to cross-verify the structure in the tarball is what we expect.

prestonvasquez · 2026-05-27T18:19:07Z

-			desc:  "deep_bson.json.gz",
-			value: readExtJSONFile("deep_bson.json.gz"),
+			desc:  "deep_bson.json",
+			value: readExtJSONFile(b, "deep_bson.json"),


[blocking] We don't need to hardcode the files, just iterate over the results of decompressing the tarball. We should rename readExtJSONFile to loadExtendedBSON. Then we should iterate over the results:

func TestLoadExtendedBSON(t *testing.T) { loadExtendedBSON(t) for filename, _ := range extJSONFiles { t.Run(filename, func(t *testing.T) { // Test / Benchmark }) } }

prestonvasquez · 2026-05-27T18:19:17Z

+// readExtJSONFile reads the named JSON file from the extended_bson.tgz archive and returns it as a
+// map[string]any. The first call decompresses the archive and caches all entries; subsequent calls
+// only look up the cache. It calls b.Fatal on any errors.
+func readExtJSONFile(b *testing.B, filename string) map[string]any {


[question] The original drivers ticket aimed to use strong types for these tests. Should we do the same? See this comment: https://github.com/mongodb/mongo-go-driver/pull/2400/changes#r3312898922

The updates of strong-typed benchmarks are in "deep_bson.json.gz". They are verified in #2403.

qingyang-hu · 2026-06-02T13:31:13Z

This change should only be applied for verification in #2403.

prestonvasquez · 2026-06-02T18:33:00Z

+	var once sync.Once
+	var onceErr error
+	entryErr := make(map[string]error)
+	results := make(map[string]map[string]any)


[blocking] These variables should global variables to avoid re-parsing the tarball.

I put them in a closure to reduce global variables. They are only invoked once before initializing loadExtendedBSON with the returned function at L161. When loadExtendedBSON is called, e.g., at L246, L250, or L254, the decompressing and parsing is performed once, which is guarded by the sync.Once.Do().

@qingyang-hu Oh I didn't see that loadExtendedBSON is acting as a package initializer. Is there a reason not to just make the variables global? The IIFE solution is hard to read. For example:

var ( loadExtendedBSONOnce sync.Once loadExtendedBSONErr error loadExtendedBSONEntries map[string]map[string]any loadExtendedBSONEntryErrs map[string]error ) func loadExtendedBSON(tb testing.TB, filename string) map[string]any { tb.Helper() loadExtendedBSONOnce.Do(func() { // ... same loading logic ... }) // ... same lookup logic ... }

GODRIVER-3868 Update extended bson cases.

3d0b335

qingyang-hu added the ci/cd label May 26, 2026

qingyang-hu marked this pull request as ready for review May 26, 2026 15:50

qingyang-hu requested a review from a team as a code owner May 26, 2026 15:50

qingyang-hu requested review from Copilot and prestonvasquez May 26, 2026 15:50

Copilot started reviewing on behalf of qingyang-hu May 26, 2026 15:51 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

qingyang-hu mentioned this pull request May 27, 2026

GODRIVER-3868 [Verification] Update benchmark suite. #2403

Draft

prestonvasquez requested changes May 27, 2026

View reviewed changes

qingyang-hu added 2 commits June 1, 2026 23:57

rewrite with sync.Once

009755f

revert changes in internal/cmd/benchmark/benchmark_test.go

b17879b

qingyang-hu commented Jun 2, 2026

View reviewed changes

qingyang-hu requested a review from prestonvasquez June 2, 2026 13:45

prestonvasquez requested changes Jun 2, 2026

View reviewed changes

	b.Fatalf("error opening %q: %s", extendedBSONTGZ, err)
	b.Fatalf("error opening %q: %v", extendedBSONTGZ, err)

	b.Fatalf("error reading tar entry %q: %s", hdr.Name, err)
	b.Fatalf("error reading tar entry %q: %v", hdr.Name, err)

	b.Fatalf("error unmarshalling %q: %s", hdr.Name, err)
	b.Fatalf("error unmarshalling %q: %v", hdr.Name, err)

Conversation

qingyang-hu commented May 26, 2026

Summary

Background & Motivation

Uh oh!

mongodb-drivers-pr-bot Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 Performance Results

Uh oh!

mongodb-drivers-pr-bot Bot commented May 26, 2026

API Change Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mongodb-drivers-pr-bot Bot commented May 26, 2026 •

edited

Loading