Generate new ObjectID only when required

[adhocore]

Summary

any insert operation preemptively generates new ObjectID to check/ensure if _id exists in doc or not - but chances are that goes to waste. this waste is more pronounced in scenarios where _id is a mix of custom userland id and system generated one across collections of a db or even across documents of same collection

snippet

type IDO struct {
    ID primitive.ObjectID `bson:"_id,omitempty"`
}

dsn := os.Getenv("MONGO_DSN")
client, _ := mongo.Connect(context.Background(), options.Client().ApplyURI(dsn))
coll := client.Database("db_name").Collection("id_test")

fmt.Println(coll.InsertOne(context.Background(), IDO{primitive.NewObjectID()}))
fmt.Println(coll.InsertOne(context.Background(), IDO{}))
fmt.Println(coll.InsertOne(context.Background(), IDO{primitive.NewObjectID()}))
fmt.Println(coll.InsertOne(context.Background(), IDO{}))
fmt.Println(coll.InsertOne(context.Background(), IDO{primitive.NewObjectID()}))
fmt.Println(coll.InsertOne(context.Background(), IDO{primitive.NewObjectID()}))
fmt.Println(coll.InsertOne(context.Background(), IDO{}))
fmt.Println(coll.InsertOne(context.Background(), IDO{}))

output before (4 wastes)

&{ObjectID("655ff7039326bc5b41e6652a")} <nil>
&{ObjectID("655ff7039326bc5b41e6652c")} <nil> // missing ***2b
&{ObjectID("655ff7039326bc5b41e6652d")} <nil>
&{ObjectID("655ff7039326bc5b41e6652f")} <nil> // missing ***2e
&{ObjectID("655ff7039326bc5b41e66530")} <nil>
&{ObjectID("655ff7039326bc5b41e66532")} <nil> // missing ***31
&{ObjectID("655ff7039326bc5b41e66534")} <nil> // missing ***33
&{ObjectID("655ff7039326bc5b41e66535")} <nil>

output after (no wastes)

&{ObjectID("655ff6d1dd10f377136f9b71")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b72")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b73")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b74")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b75")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b76")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b77")} <nil>
&{ObjectID("655ff6d1dd10f377136f9b78")} <nil>

Background & Motivation

ID burning is a thing in sql world, while this one is different world here, i believe it does not hurt to be cautious of resource we use/disuse. quoting from summary above:

any insert operation preemptively generates new ObjectID to check/ensure if _id exists in doc or not - but chances are that goes to waste.

isn't it more logical to generate it if only we ever really need it?

[adhocore]

any plans on the fate of this PR? thanks

[mongodb-drivers-pr-bot]

API Change Report

No changes found!

[prestonvasquez]

Hi @adhocore , thank you for this contribution. I noticed when testing this that we do not have a benchmark for NewObjectID(). Would mind adding one in the bson/primitive/object_test.go file?

func BenchmarkNewObjectIDFromTimestamp(b *testing.B) {
	for i := 0; i < b.N; i++ {
		timestamp := time.Now().Add(time.Duration(i) * time.Millisecond)
		_ = NewObjectIDFromTimestamp(timestamp)
	}
}

We should also update the comment for ensureID to match the new behavior:

// ensureID inserts the given ObjectID as an element named "_id" at the
// beginning of the given BSON document if there is not an "_id" already. If
// the given ObjectID is primitive.NilObjectID, a new object ID will be
// generated with time.Now().
//
// If there is already an element named "_id", the document is not modified. It
// returns the resulting document and the decoded Go value of the "_id" element.

Also, we would like to test this use case more closely. There already exists a test for ensureID, found here. However, it's not quite written to deal with this change. So I would suggest adding another test validating this specific usage:

func TestEnsureID_NilObjectID(t *testing.T) {
	t.Parallel()

	doc := bsoncore.NewDocumentBuilder().
		AppendString("foo", "bar").
		Build()

	got, gotIDI, err := ensureID(doc, primitive.NilObjectID, nil, nil)
	assert.NoError(t, err)

	gotID, ok := gotIDI.(primitive.ObjectID)

	assert.True(t, ok)
	assert.NotEqual(t, primitive.NilObjectID, gotID)

	want := bsoncore.NewDocumentBuilder().
		AppendObjectID("_id", gotID).
		AppendString("foo", "bar").
		Build()

	assert.Equal(t, want, got)
}

[adhocore]

thanks for feedback. will take a look into that asap :)

[adhocore]

updated accordingly. the TestEnsureID_NilObjectID test run looks like:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^TestEnsureID_NilObjectID$ go.mongodb.org/mongo-driver/mongo

ok  	go.mongodb.org/mongo-driver/mongo	(cached)

and the benchmark looks like:

Running tool: /usr/local/go/bin/go test -benchmem -run=^$ -bench ^BenchmarkNewObjectIDFromTimestamp$ go.mongodb.org/mongo-driver/bson/primitive

goos: linux
goarch: amd64
pkg: go.mongodb.org/mongo-driver/bson/primitive
cpu: 12th Gen Intel(R) Core(TM) i9-12900H
BenchmarkNewObjectIDFromTimestamp-20    	 7597987	       157.9 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	go.mongodb.org/mongo-driver/bson/primitive	1.368s

[prestonvasquez]

LGTM!