Skip to content

Commit 2ffc7c8

Browse files
jyeminjyemin
committed
JAVA-2303: Improve performance of LazyBSONObject.entrySet and hashCode
 The root cause of the performance problem was that the entrySet implementation inserted all the entries into an actual HashSet, which in turn called hashCode on all embedded LazyBSONOBject instances. The hashCode implemenation examined every byte in the array instead of just the bytes between offset and size. The fix is two-fold. The first is to use a private Set implementation for the entrySet that just wraps an ArrayList and therefore doesn't need to call hashCode anymore at all. While calls to entrySet().contains and entrySet().containsAll will be slower, that is an acceptable trade-off as usage of those methods is likely to be rare. The second change is to ensure that LazyBSONObject.hashCode only examines the bytes between offset and size, just in case clients are sticking instances in their own hash tables.
1 parent a6ba15a commit 2ffc7c8

File tree

2 files changed

+162
-20
lines changed

2 files changed

+162
-20
lines changed

‎driver/src/main/org/bson/LazyBSONObject.java

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,14 @@
3232
import java.nio.channels.Channels;
3333
import java.nio.channels.WritableByteChannel;
3434
import java.util.AbstractMap;
35-
import java.util.Arrays;
35+
import java.util.ArrayList;
36+
import java.util.Collection;
3637
import java.util.Collections;
3738
import java.util.Date;
39+
import java.util.Iterator;
3840
import java.util.LinkedHashMap;
3941
import java.util.LinkedHashSet;
42+
import java.util.List;
4043
import java.util.Map;
4144
import java.util.Set;
4245
import java.util.UUID;
@@ -203,7 +206,7 @@ Object readValue(final BsonBinaryReader reader) {
203206
case SYMBOL:
204207
return new Symbol(reader.readSymbol());
205208
case JAVASCRIPT_WITH_SCOPE:
206-
return new CodeWScope(reader.readJavaScriptWithScope(), (BSONObject) readDocument(reader));
209+
return new CodeWScope(reader.readJavaScriptWithScope(), (BSONObject) readJavaScriptWithScopeDocument(reader));
207210
case INT32:
208211
return reader.readInt32();
209212
case TIMESTAMP:
@@ -231,6 +234,12 @@ private Object readArray(final BsonBinaryReader reader) {
231234
}
232235

233236
private Object readDocument(final BsonBinaryReader reader) {
237+
int position = reader.getBsonInput().getPosition();
238+
reader.skipValue();
239+
return callback.createObject(bytes, offset + position);
240+
}
241+
242+
private Object readJavaScriptWithScopeDocument(final BsonBinaryReader reader) {
234243
int position = reader.getBsonInput().getPosition();
235244
reader.readStartDocument();
236245
while (reader.readBsonType() != BsonType.END_OF_DOCUMENT) {
@@ -285,12 +294,12 @@ public int pipe(final OutputStream os) throws IOException {
285294
}
286295

287296
/**
288-
* Gets the entry set for all the key/value pairs in this {@code BSONObject}.
297+
* Gets the entry set for all the key/value pairs in this {@code BSONObject}. The returned set is immutable.
289298
*
290299
* @return then entry set
291300
*/
292301
public Set<Map.Entry<String, Object>> entrySet() {
293-
Set<Map.Entry<String, Object>> entries = new LinkedHashSet<Map.Entry<String, Object>>();
302+
final List<Map.Entry<String, Object>> entries = new ArrayList<Map.Entry<String, Object>>();
294303
BsonBinaryReader reader = getBsonReader();
295304
try {
296305
reader.readStartDocument();
@@ -301,12 +310,82 @@ public Set<Map.Entry<String, Object>> entrySet() {
301310
} finally {
302311
reader.close();
303312
}
304-
return Collections.unmodifiableSet(entries);
313+
return new Set<Map.Entry<String, Object>>() {
314+
@Override
315+
public int size() {
316+
return entries.size();
317+
}
318+
319+
@Override
320+
public boolean isEmpty() {
321+
return entries.isEmpty();
322+
}
323+
324+
@Override
325+
public Iterator<Map.Entry<String, Object>> iterator() {
326+
return entries.iterator();
327+
}
328+
329+
@Override
330+
public Object[] toArray() {
331+
return entries.toArray();
332+
}
333+
334+
@Override
335+
public <T> T[] toArray(final T[] a) {
336+
return entries.toArray(a);
337+
}
338+
339+
@Override
340+
public boolean contains(final Object o) {
341+
return entries.contains(o);
342+
}
343+
344+
@Override
345+
public boolean containsAll(final Collection<?> c) {
346+
return entries.containsAll(c);
347+
}
348+
349+
@Override
350+
public boolean add(final Map.Entry<String, Object> stringObjectEntry) {
351+
throw new UnsupportedOperationException();
352+
}
353+
354+
@Override
355+
public boolean remove(final Object o) {
356+
throw new UnsupportedOperationException();
357+
}
358+
359+
@Override
360+
public boolean addAll(final Collection<? extends Map.Entry<String, Object>> c) {
361+
throw new UnsupportedOperationException();
362+
}
363+
364+
@Override
365+
public boolean retainAll(final Collection<?> c) {
366+
throw new UnsupportedOperationException();
367+
}
368+
369+
@Override
370+
public boolean removeAll(final Collection<?> c) {
371+
throw new UnsupportedOperationException();
372+
}
373+
374+
@Override
375+
public void clear() {
376+
throw new UnsupportedOperationException();
377+
}
378+
};
305379
}
306380

307381
@Override
308382
public int hashCode() {
309-
return Arrays.hashCode(bytes);
383+
int result = 1;
384+
int size = getBSONSize();
385+
for (int i = offset; i < offset + size; i++) {
386+
result = 31 * result + bytes[i];
387+
}
388+
return result;
310389
}
311390

312391
@Override

‎driver/src/test/unit/org/bson/LazyBSONObjectSpecification.groovy

Lines changed: 77 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,18 +152,28 @@ class LazyBSONObjectSpecification extends Specification {
152152
]
153153

154154
when:
155-
LazyBSONObject bsonObject1 = new LazyBSONObject(bytes, new LazyBSONCallback())
156-
LazyBSONObject bsonObject2 = new LazyBSONObject(bytes, new LazyBSONCallback())
157-
LazyBSONObject bsonObject3 = new LazyBSONObject(bytes, 7, new LazyBSONCallback())
158-
LazyBSONObject bsonObject4 = new LazyBSONObject(bytes, 24, new LazyBSONCallback())
159-
155+
def bsonObject1 = new LazyBSONObject(bytes, new LazyBSONCallback())
156+
def bsonObject2 = new LazyBSONObject(bytes, new LazyBSONCallback())
157+
def bsonObject3 = new LazyBSONObject(bytes, 7, new LazyBSONCallback())
158+
def bsonObject4 = new LazyBSONObject(bytes, 24, new LazyBSONCallback())
159+
def bsonObject5 = new LazyBSONObject([14, 0, 0, 0, 2, 120, 0, 2, 0, 0, 0, 121, 0, 0] as byte[], new LazyBSONCallback())
160+
def bsonObject6 = new LazyBSONObject([5, 0, 0, 0, 0] as byte[], new LazyBSONCallback())
160161

161162
then:
162-
bsonObject1 == bsonObject2
163-
bsonObject3 == bsonObject4
164-
bsonObject1 != bsonObject3
165-
bsonObject4 == new LazyBSONObject([14, 0, 0, 0, 2, 120, 0, 2, 0, 0, 0, 121, 0, 0] as byte[], new LazyBSONCallback())
166-
bsonObject1 != new LazyBSONObject([] as byte[], new LazyBSONCallback())
163+
bsonObject1.equals(bsonObject1)
164+
!bsonObject1.equals(null)
165+
!bsonObject1.equals('not equal')
166+
bsonObject1.equals(bsonObject2)
167+
bsonObject3.equals(bsonObject4)
168+
!bsonObject1.equals(bsonObject3)
169+
bsonObject4.equals(bsonObject5)
170+
!bsonObject1.equals(bsonObject6)
171+
172+
bsonObject1.hashCode() == bsonObject2.hashCode()
173+
bsonObject3.hashCode() == bsonObject4.hashCode()
174+
bsonObject1.hashCode() != bsonObject3.hashCode()
175+
bsonObject4.hashCode() == bsonObject5.hashCode()
176+
bsonObject1.hashCode() != bsonObject6.hashCode()
167177
}
168178

169179
def 'should return the size of a document'() {
@@ -205,14 +215,67 @@ class LazyBSONObjectSpecification extends Specification {
205215
def 'should implement Map.entrySet()'() {
206216
given:
207217
byte[] bytes = [16, 0, 0, 0, 16, 97, 0, 1, 0, 0, 0, 8, 98, 0, 1, 0]
218+
LazyBSONObject document = new LazyBSONObject(bytes, new LazyBSONCallback())
208219

209220
when:
210-
LazyBSONObject document = new LazyBSONObject(bytes, new LazyBSONCallback())
221+
def entrySet = document.entrySet()
222+
223+
then:
224+
entrySet.size() == 2
225+
!entrySet.isEmpty()
226+
entrySet.contains(new AbstractMap.SimpleImmutableEntry('a', 1))
227+
!entrySet.contains(new AbstractMap.SimpleImmutableEntry('a', 2))
228+
entrySet.containsAll([new AbstractMap.SimpleImmutableEntry('a', 1), new AbstractMap.SimpleImmutableEntry('b', true)])
229+
!entrySet.containsAll([new AbstractMap.SimpleImmutableEntry('a', 1), new AbstractMap.SimpleImmutableEntry('b', false)])
230+
entrySet.toArray() == [new AbstractMap.SimpleImmutableEntry('a', 1), new AbstractMap.SimpleImmutableEntry('b', true)].toArray()
231+
entrySet.toArray(new Map.Entry[2]) ==
232+
[new AbstractMap.SimpleImmutableEntry('a', 1), new AbstractMap.SimpleImmutableEntry('b', true)].toArray()
233+
234+
when:
235+
def iterator = entrySet.iterator()
236+
237+
then:
238+
iterator.hasNext()
239+
iterator.next() == new AbstractMap.SimpleImmutableEntry('a', 1)
240+
iterator.hasNext()
241+
iterator.next() == new AbstractMap.SimpleImmutableEntry('b', true)
242+
!iterator.hasNext()
243+
244+
when:
245+
entrySet.add(new AbstractMap.SimpleImmutableEntry('key', null))
246+
247+
then:
248+
thrown(UnsupportedOperationException)
249+
250+
when:
251+
entrySet.addAll([new AbstractMap.SimpleImmutableEntry('key', null)])
252+
253+
then:
254+
thrown(UnsupportedOperationException)
255+
256+
when:
257+
entrySet.clear()
258+
259+
then:
260+
thrown(UnsupportedOperationException)
261+
262+
when:
263+
entrySet.remove(new AbstractMap.SimpleImmutableEntry('key', null))
211264

212265
then:
213-
document.entrySet().size() == 2
214-
document.entrySet().find { it.value == 1 } != null
215-
document.entrySet().find { it.value == true } != null
266+
thrown(UnsupportedOperationException)
267+
268+
when:
269+
entrySet.removeAll([new AbstractMap.SimpleImmutableEntry('key', null)])
270+
271+
then:
272+
thrown(UnsupportedOperationException)
273+
274+
when:
275+
entrySet.retainAll([new AbstractMap.SimpleImmutableEntry('key', null)])
276+
277+
then:
278+
thrown(UnsupportedOperationException)
216279
}
217280

218281
def 'should throw on modification'() {

0 commit comments

Comments
 (0)