[JAVA-3328] Excessive memory allocation due to unwrapping of updates in bulk ops Created: 18/Jun/19  Updated: 28/Oct/23  Resolved: 24/Jul/19

Status: Closed
Project: Java Driver
Component/s: Write Operations
Affects Version/s: 3.6.0, 3.7.0, 3.8.0, 3.9.0, 3.10.0
Fix Version/s: 3.11.0

Type: Bug Priority: Major - P3
Reporter: Oleg Rekutin Assignee: Jeffrey Yemin
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screen Shot 2019-06-18 at 5.21.28 PM.png    
Backwards Compatibility: Fully Compatible

 Description   

It looks like the changes inĀ 332ffc78538b36db880c94836d82276f5e5d9296 introduced a call to isEmpty() on the update document, to throw an illegal arg exception if the update document is empty.

However, when used with BsonDocumentWrapper, the call to isEmpty() causes the wrapped document to be unwrapped, just to check for emptiness. This causes many unnecessary allocations when unwrapping update documents of any size.

Removing this call saves about approx ~80% GC'd allocations for a comparable volume of writes, as illustrated in the attached screenshot comparing two JProfiler snapshots. On the left is the version w/o the check, on the right is the current code w/ the check.



 Comments   
Comment by Githook User [ 01/Aug/19 ]

Author:

{'name': 'Jeff Yemin', 'email': 'jeff.yemin@10gen.com', 'username': 'jyemin'}

Message: Optimize check for an empty update document

The CRUD specification requires that an update contains at least one
field, in order to check for application bugs. Because once the command
makes it to the server it can't be distinguished from a replace (where
an empty document is legal).

The current method of checking for an empty document is to call
BsonDocument#isEmpty. But that has the unfortunate affect of hydrating
a BsonDocumentWrapper into a full BsonDocument. We really want to avoid
that hit. To do so, we need another way of checking for an empty
document. We use the technique of wrapping the BsonWriter with a
FieldTracking BsonWriter, which tracks whether at least one field
is written. BulkWriteBatch then checks that and throws if it's not.

This also fixes a bug in which that check was done for both update and
replace requests. An empty replace document, while rare, is legal.

Thanks to @astral303 for finding this issue and for opening an initial
PR that provided inspiration for this fix.

JAVA-3328
Branch: mongot
https://github.com/mongodb/mongo-java-driver/commit/6d207815a4e627abe8227791837c9746d45b78f2

Comment by Githook User [ 01/Aug/19 ]

Author:

{'name': 'Jeff Yemin', 'email': 'jeff.yemin@10gen.com', 'username': 'jyemin'}

Message: Introduce BsonWriterDecorator superclass

JAVA-3328
Branch: mongot
https://github.com/mongodb/mongo-java-driver/commit/b744eff936e090de931b0f7713437881cda8bb14

Comment by Githook User [ 24/Jul/19 ]

Author:

{'name': 'Jeff Yemin', 'username': 'jyemin', 'email': 'jeff.yemin@10gen.com'}

Message: Optimize check for an empty update document

The CRUD specification requires that an update contains at least one
field, in order to check for application bugs. Because once the command
makes it to the server it can't be distinguished from a replace (where
an empty document is legal).

The current method of checking for an empty document is to call
BsonDocument#isEmpty. But that has the unfortunate affect of hydrating
a BsonDocumentWrapper into a full BsonDocument. We really want to avoid
that hit. To do so, we need another way of checking for an empty
document. We use the technique of wrapping the BsonWriter with a
FieldTracking BsonWriter, which tracks whether at least one field
is written. BulkWriteBatch then checks that and throws if it's not.

This also fixes a bug in which that check was done for both update and
replace requests. An empty replace document, while rare, is legal.

Thanks to @astral303 for finding this issue and for opening an initial
PR that provided inspiration for this fix.

JAVA-3328
Branch: master
https://github.com/mongodb/mongo-java-driver/commit/6d207815a4e627abe8227791837c9746d45b78f2

Comment by Githook User [ 24/Jul/19 ]

Author:

{'name': 'Jeff Yemin', 'username': 'jyemin', 'email': 'jeff.yemin@10gen.com'}

Message: Introduce BsonWriterDecorator superclass

JAVA-3328
Branch: master
https://github.com/mongodb/mongo-java-driver/commit/b744eff936e090de931b0f7713437881cda8bb14

Comment by Oleg Rekutin [ 18/Jun/19 ]

Added pull request and a benchmark you can run yourself w/ memory allocation profiling.
https://github.com/mongodb/mongo-java-driver/pull/515

Generated at Thu Feb 08 08:59:20 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.