I've been looking at some profiles of the performance test, and one of
the bottlenecks occurs because of this code in ObjectBuilder.java:
public void iterate(DescriptorIterator iterator) {
// PERF: Avoid synchronized enumerator as is concurrency bottleneck.
Vector mappings = getDescriptor().getMappings();
for (int index = 0; index < mappings.size(); index++) {
((DatabaseMapping)mappings.get(index)).iterate(iterator);
}
}
I'm not sure of the intent behind this code, but it is either
sub-optimal or semantically incorrect -- depending on the answers to
these questions:
1) Is the Database mapping vector immutable? If so, then at the very
least the mappings.size() can be pulled out of the loop -- but really, a
simple array would be a much more efficient data structure. If not, then
the next questions come into play:
2) Are items removed from the mapping (one thread removes while another
thread is iterating)? If so, this method will sometimes throw an
ArrayIndexOutOfBoundsException. [Which conceivably could happen also if
an item is added given alternate implementations of the Vector class,
and of course we can't depend on the implementation of that class.]
Additionally, it may skip items (which may or may not be a semantic
error).
3) Are items added in the middle of the mapping? If so, the iterator
will process some items twice (which may or may not be a semantic error
but probably isn't good for performance). [If items are appended to the
mapping, they may be skipped by this iterator -- again, possibly a
semantic error but not necessarily.]
Most interesting to me is that we got to this code because when the
current transaction ended, the toplink code when through
collectAndPrepareForCommit() -> assignSequenceNumbers() ->
discoverAllUnregisteredNewObjects(). But as this is a read-only test,
searching a two-dimensional array for something that won't be there is
something that could be optimized. Though that's doubtless a long-term
thing to look at.
In the meanwhile, can someone explain to me the usage of the data
structure in this context (and, in fact, similar code in
DefferredChangeDetectonPolicy.createObjectChangeSetThroughComparison(),
DatabaseAccessor.fetchRow(), ObjectBuilder.buildAttributesIntoObject(),
and AbtractRecord.getIndicatingNoEntry() -- call from which to
vector.size() account for 6% of the time in the profile, primarily from
synchronization contention).
-Scott