[CONTROLLER-1871] Excessive Optional.ofNullable() from QName/QNameModule.getRevision() from NormalizedNodeOutputStreamWriter Created: 13/Nov/18  Updated: 14/Nov/18  Resolved: 14/Nov/18

Status: Resolved
Project: controller
Component/s: None
Affects Version/s: Oxygen SR3
Fix Version/s: Neon, Oxygen SR4, Fluorine SR2

Type: Bug Priority: Medium
Reporter: Michael Vorburger Assignee: Tom Pantelis
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

I'm looking at a Java Flight Recording obtained from (internal) scale lab testing, based on Oxygen SR3 code, and see a lot (cumulated as in millions, with many tens of GBs) of "TLAB Allocations" related to an Optional.of() Optional.ofNullable() from QName/QNameModule.getRevision() from NormalizedNodeOutputStreamWriter.

Wondering if perhaps we could improve upon this in any way? Realizing this may not be trivial, and generally speaking Optional is much preferred over using a @Nullable, but wondering if this is an exceptional case where the Optional should perhaps be avoided?

Optional java.util.Optional.of(Object)	36282
Optional java.util.Optional.ofNullable(Object)	31841
Optional org.opendaylight.yangtools.yang.common.QNameModule.getRevision()	11703
Optional org.opendaylight.yangtools.yang.common.QName.getRevision()	11703
void org.opendaylight.controller.cluster.datastore.node.utils.stream.NormalizedNodeOutputStreamWriter.writeQName(QName)	11703
void org.opendaylight.controller.cluster.datastore.node.utils.stream.AbstractNormalizedNodeDataOutput.startNode(QName, byte)	8215
void org.opendaylight.controller.cluster.datastore.node.utils.stream.AbstractNormalizedNodeDataOutput.leafNode(YangInstanceIdentifier$NodeIdentifier, Object)	5324
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.wasProcessAsSimpleNode(NormalizedNode)	5324
NormalizedNodeWriter org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.write(NormalizedNode)	5324
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.writeChildren(Iterable)	4596
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.wasProcessedAsCompositeNode(NormalizedNode)	3924
NormalizedNodeWriter org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.write(NormalizedNode)	3924
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.writeChildren(Iterable)	2301
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.wasProcessedAsCompositeNode(NormalizedNode)	2284
NormalizedNodeWriter org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.write(NormalizedNode)	2284
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.writeChildren(Iterable)	1759
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter$OrderedNormalizedNodeWriter.writeMapEntryNode(MapEntryNode)	1485
boolean org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.wasProcessedAsCompositeNode(NormalizedNode)	1485
NormalizedNodeWriter org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter.write(NormalizedNode)	1485
void org.opendaylight.controller.cluster.datastore.node.utils.stream.AbstractNormalizedNodeDataOutput.writeNormalizedNode(NormalizedNode)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeNode(NormalizedNodeDataOutput, DataTreeCandidateNode)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeChildren(NormalizedNodeDataOutput, Collection)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeNode(NormalizedNodeDataOutput, DataTreeCandidateNode)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeChildren(NormalizedNodeDataOutput, Collection)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeNode(NormalizedNodeDataOutput, DataTreeCandidateNode)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeChildren(NormalizedNodeDataOutput, Collection)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeNode(NormalizedNodeDataOutput, DataTreeCandidateNode)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeChildren(NormalizedNodeDataOutput, Collection)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeNode(NormalizedNodeDataOutput, DataTreeCandidateNode)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeChildren(NormalizedNodeDataOutput, Collection)	1023
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeNode(NormalizedNodeDataOutput, DataTreeCandidateNode)	1018
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeChildren(NormalizedNodeDataOutput, Collection)	1018
void org.opendaylight.controller.cluster.datastore.persisted.DataTreeCandidateInputOutput.writeDataTreeCandidate(DataOutput, DataTreeCandidate)	1018
CommitTransactionPayload org.opendaylight.controller.cluster.datastore.persisted.CommitTransactionPayload.create(TransactionIdentifier, DataTreeCandidate)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree.startCommit(SimpleShardDataTreeCohort, DataTreeCandidate)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.commit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.CohortEntry.commit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator.finishCommit(ActorRef, CohortEntry)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator$3.onSuccess(DataTreeCandidate)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator$3.onSuccess(Object)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.successfulPreCommit(DataTreeCandidateTip)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree$1.onSuccess(Void)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree$1.onSuccess(Object)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.doUserPreCommit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.userPreCommit(DataTreeCandidate, FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree.startPreCommit(SimpleShardDataTreeCohort)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.preCommit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.CohortEntry.preCommit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator.doCommit(CohortEntry)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator$2.onSuccess(Void)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator$2.onSuccess(Object)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.successfulCanCommit()	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree.lambda$processNextPendingTransaction$0(ShardDataTree$CommitEntry)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree$$Lambda$863.1527042624.accept(Object)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree.processNextPending(Queue, ShardDataTreeCohort$State, Consumer)	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree.processNextPendingTransaction()	1018
void org.opendaylight.controller.cluster.datastore.ShardDataTree.startCanCommit(SimpleShardDataTreeCohort)	1018
void org.opendaylight.controller.cluster.datastore.SimpleShardDataTreeCohort.canCommit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.CohortEntry.canCommit(FutureCallback)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator.handleCanCommit(CohortEntry)	1018
void org.opendaylight.controller.cluster.datastore.ShardCommitCoordinator.handleReadyLocalTransaction(ReadyLocalTransaction, ActorRef, Shard)	1018
void org.opendaylight.controller.cluster.datastore.Shard.handleReadyLocalTransaction(ReadyLocalTransaction)	1018
void org.opendaylight.controller.cluster.datastore.Shard.handleNonRaftCommand(Object)	1018
void org.opendaylight.controller.cluster.raft.RaftActor.handleCommand(Object)	1018

 Comments   
Comment by Tom Pantelis [ 13/Nov/18 ]

The Optional comes from QName so this should really be in yangtools. Perhaps QName could provide an @Nullable version of getRevision or store the revision as an Optional - of course that would increase the footprint but QName instances are reused so perhaps that wouldn't be significant (rovarga).

That said, the Optionals in this case are short-lived so really shouldn't have a significant affect on GC.

Comment by Robert Varga [ 13/Nov/18 ]

We are using Optional for return values, which are immediately handled – making them immediate candidates for on-stack allocation and (if not) extremely easy to collect. They certainly do not end up leaking out of the current thread.

Storing an Optional in QName is not really a good idea until we have Valhalla-capable JDK, as it adds 16 bytes of memory overhead for each QName.

Comment by Tom Pantelis [ 13/Nov/18 ]

+1

Comment by Robert Varga [ 13/Nov/18 ]

"many tens of GB" ... over what period of time?

Comment by Michael Vorburger [ 13/Nov/18 ]

The scale test where this is from ran for only about 5h. IRL it's much worse.

Comment by Robert Varga [ 13/Nov/18 ]

Well, according to https://shipilev.net/jvm-anatomy-park/4-tlab-allocation/ , TLAB can easily perform allocations to the tune of 2.5GB/s – making the overhead 'couple of seconds in a 5 hour run'.

Comment by Robert Varga [ 14/Nov/18 ]

This is a reasonable use of API design. We will not go back to nullables.

Generated at Wed Feb 07 19:56:39 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.