Re: Slow performance in AbstractNodeState.equals
Hi Angela, yes, I saw it. Forgot to ping them myself as you suggested. Thanks for helping me out here. Best regards, Alex Am 29.07.2020 um 18:35 schrieb Angela Schreiber: Hi Alexander Thanks for creating a JIRA ticket. I added a comment mentioning those committers that to my knowledge have been working on this and have insight in order to review your report and patch. Hope that helps Angela From: Alexander Lüders Sent: Wednesday, July 29, 2020 12:08 PM To: oak-dev@jackrabbit.apache.org Subject: Re: Slow performance in AbstractNodeState.equals Hi Angela, thanks for taking the time to respond. I did as you suggested and created a ticket here: https://issues.apache.org/jira/browse/OAK-9158 Best regards, Alex Am 28.07.2020 um 10:10 schrieb Angela Schreiber: Hi Alexander Thanks for the detailed report and analysis. May I suggest that you create a ticket at https://issues.apache.org/jira/projects/OAK/issues <https://issues.apache.org/jira/projects/OAK/issues> with component documentmk providing all the details and attaching the patch? I think that's the best way to move forward. Unfortunately, I am not sufficiently familiar with that area of Oak to review the proposed modification, but Marcel Reutegger and Julian Reschke will for sure be able to provide you with feedback and suggestions and I would suggest you ping them directly in the Jira ticket. Kind regards Angela *From:* Alexander Lüders *Sent:* Friday, July 24, 2020 3:19 PM *To:* oak-dev@jackrabbit.apache.org *Subject:* Re: Slow performance in AbstractNodeState.equals Hi again, digging deeper I derived following patch which solved the performance issue: Index: oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 === --- oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (revision 17ea60b0bfb0ca3670c99208170a774a6c99fdfc) +++ oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (date 1595575140063) @@ -83,7 +83,11 @@ // revision does not match: might still be equals } else if (that instanceof ModifiedNodeState) { ModifiedNodeState modified = (ModifiedNodeState) that; -if (modified.getBaseState() == this) { +NodeState baseState = modified.getBaseState(); +if (baseState instanceof ModifiedDocumentNodeState) { +baseState = ((ModifiedDocumentNodeState) baseState).getBaseState(); +} +if (baseState == this) { return EqualsDiff.equals(this, modified); } } I am not entirely sure that this is correct but comparing with the /ModifiedDocumentNodeState#equals/ method, I assume it to be. The latter method compares the passed object with its baseState not the /ModifiedDocumentNodeState/ instance itself. This is the prime difference when comparing the differences of /ModifiedDocumentNodeState#equals/ and /AbstractNodeDocumentState#equals/. The patch above transfers that logic to /AbstractNodeDocumentState#equals/. What do you think? Many thanks for your help. Best regards, Alexander Lüders Am 23.07.2020 um 17:26 schrieb Alexander Lüders: Dear Jackrabbit Oak developers, we are currently trying to hunt down a performance issue we are facing at a customer site. We strongly need your help on this so any comments would be greatly appreciated. We have a MongoDB as a backend of a Oak repository where we see hundreds of expensive queries being executed during a rather simple operation. It happens when we are saving a session in which we added a child node to a node already containing a very large number of children. This has not been the case with Jackrabbit Oak 1.4.8 and 1.4.26 but it is an issue with 1.10.8 and 1.22.3. Newer versions have not been tested yet but looking at the source we believe that they will show that performance issue too. What we know so far: * The queries against the /MongoDocumentStore /are triggered via /AbstractNodeState.equals/ (see stacktrace below) * A TODO "inefficient unless there are very few child nodes" in that method indicates poor performance but this has been there since a long time and cannot explain the issue * OAK-7401 introduced the class /ModifiedDocumentNodeState /class which is not inheriting /ModifiedNodeState /or /AbstractDocumentNodeState /(see class inheritance diagram) */AbstractDocumentNodeState#equals/ is checking against instances of /ModifiedNodeState /and /AbstractDocumentNodeState /but not /ModifiedDocumentNodeState/. * /AbstractDocumentNodeState#equals/ triggers the "slow" /AbstractNodeSt
Re: Slow performance in AbstractNodeState.equals
Hi Alexander Thanks for creating a JIRA ticket. I added a comment mentioning those committers that to my knowledge have been working on this and have insight in order to review your report and patch. Hope that helps Angela From: Alexander Lüders Sent: Wednesday, July 29, 2020 12:08 PM To: oak-dev@jackrabbit.apache.org Subject: Re: Slow performance in AbstractNodeState.equals Hi Angela, thanks for taking the time to respond. I did as you suggested and created a ticket here: https://issues.apache.org/jira/browse/OAK-9158 Best regards, Alex Am 28.07.2020 um 10:10 schrieb Angela Schreiber: > Hi Alexander > > Thanks for the detailed report and analysis. > May I suggest that you create a ticket at > https://issues.apache.org/jira/projects/OAK/issues > <https://issues.apache.org/jira/projects/OAK/issues> with component > documentmk providing all the details and attaching the patch? I think > that's the best way to move forward. > > Unfortunately, I am not sufficiently familiar with that area of Oak to > review the proposed modification, but Marcel Reutegger and Julian > Reschke will for sure be able to provide you with feedback and > suggestions and I would suggest you ping them directly in the Jira ticket. > > Kind regards > Angela > > > > *From:* Alexander Lüders > *Sent:* Friday, July 24, 2020 3:19 PM > *To:* oak-dev@jackrabbit.apache.org > *Subject:* Re: Slow performance in AbstractNodeState.equals > Hi again, > > digging deeper I derived following patch which solved the performance > issue: > > Index: > oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java > IDEA additional info: > Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP > <+>UTF-8 > === > --- > oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java > (revision 17ea60b0bfb0ca3670c99208170a774a6c99fdfc) > +++ > oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java > (date 1595575140063) > @@ -83,7 +83,11 @@ > // revision does not match: might still be equals > } else if (that instanceof ModifiedNodeState) { > ModifiedNodeState modified = (ModifiedNodeState) that; > -if (modified.getBaseState() == this) { > +NodeState baseState = modified.getBaseState(); > +if (baseState instanceof ModifiedDocumentNodeState) { > +baseState = ((ModifiedDocumentNodeState) > baseState).getBaseState(); > +} > +if (baseState == this) { > return EqualsDiff.equals(this, modified); > } > } > > I am not entirely sure that this is correct but comparing with the > /ModifiedDocumentNodeState#equals/ method, I assume it to be. > The latter method compares the passed object with its baseState not > the /ModifiedDocumentNodeState/ instance itself. This is the prime > difference when comparing the differences of > /ModifiedDocumentNodeState#equals/ and /AbstractNodeDocumentState#equals/. > The patch above transfers that logic to > /AbstractNodeDocumentState#equals/. > > What do you think? > > Many thanks for your help. > Best regards, > Alexander Lüders > > Am 23.07.2020 um 17:26 schrieb Alexander Lüders: >> Dear Jackrabbit Oak developers, >> >> we are currently trying to hunt down a performance issue we are >> facing at a customer site. >> We strongly need your help on this so any comments would be greatly >> appreciated. >> >> We have a MongoDB as a backend of a Oak repository where we see >> hundreds of expensive queries being executed during a rather simple >> operation. >> It happens when we are saving a session in which we added a child >> node to a node already containing a very large number of children. >> This has not been the case with Jackrabbit Oak 1.4.8 and 1.4.26 but >> it is an issue with 1.10.8 and 1.22.3. >> Newer versions have not been tested yet but looking at the source we >> believe that they will show that performance issue too. >> >> What we know so far: >> >> * The queries against the /MongoDocumentStore /are triggered via >> /AbstractNodeState.equals/ (see stacktrace below) >> * A TODO "inefficient unless there are very few child nodes" in that >> method indicates poor performance but this has been there since a >> long time and cannot explain the issue >> * OAK-7401 introduc
Re: Slow performance in AbstractNodeState.equals
Hi Angela, thanks for taking the time to respond. I did as you suggested and created a ticket here: https://issues.apache.org/jira/browse/OAK-9158 Best regards, Alex Am 28.07.2020 um 10:10 schrieb Angela Schreiber: Hi Alexander Thanks for the detailed report and analysis. May I suggest that you create a ticket at https://issues.apache.org/jira/projects/OAK/issues <https://issues.apache.org/jira/projects/OAK/issues> with component documentmk providing all the details and attaching the patch? I think that's the best way to move forward. Unfortunately, I am not sufficiently familiar with that area of Oak to review the proposed modification, but Marcel Reutegger and Julian Reschke will for sure be able to provide you with feedback and suggestions and I would suggest you ping them directly in the Jira ticket. Kind regards Angela *From:* Alexander Lüders *Sent:* Friday, July 24, 2020 3:19 PM *To:* oak-dev@jackrabbit.apache.org *Subject:* Re: Slow performance in AbstractNodeState.equals Hi again, digging deeper I derived following patch which solved the performance issue: Index: oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 === --- oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (revision 17ea60b0bfb0ca3670c99208170a774a6c99fdfc) +++ oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (date 1595575140063) @@ -83,7 +83,11 @@ // revision does not match: might still be equals } else if (that instanceof ModifiedNodeState) { ModifiedNodeState modified = (ModifiedNodeState) that; - if (modified.getBaseState() == this) { + NodeState baseState = modified.getBaseState(); + if (baseState instanceof ModifiedDocumentNodeState) { + baseState = ((ModifiedDocumentNodeState) baseState).getBaseState(); + } + if (baseState == this) { return EqualsDiff.equals(this, modified); } } I am not entirely sure that this is correct but comparing with the /ModifiedDocumentNodeState#equals/ method, I assume it to be. The latter method compares the passed object with its baseState not the /ModifiedDocumentNodeState/ instance itself. This is the prime difference when comparing the differences of /ModifiedDocumentNodeState#equals/ and /AbstractNodeDocumentState#equals/. The patch above transfers that logic to /AbstractNodeDocumentState#equals/. What do you think? Many thanks for your help. Best regards, Alexander Lüders Am 23.07.2020 um 17:26 schrieb Alexander Lüders: Dear Jackrabbit Oak developers, we are currently trying to hunt down a performance issue we are facing at a customer site. We strongly need your help on this so any comments would be greatly appreciated. We have a MongoDB as a backend of a Oak repository where we see hundreds of expensive queries being executed during a rather simple operation. It happens when we are saving a session in which we added a child node to a node already containing a very large number of children. This has not been the case with Jackrabbit Oak 1.4.8 and 1.4.26 but it is an issue with 1.10.8 and 1.22.3. Newer versions have not been tested yet but looking at the source we believe that they will show that performance issue too. What we know so far: * The queries against the /MongoDocumentStore /are triggered via /AbstractNodeState.equals/ (see stacktrace below) * A TODO "inefficient unless there are very few child nodes" in that method indicates poor performance but this has been there since a long time and cannot explain the issue * OAK-7401 introduced the class /ModifiedDocumentNodeState /class which is not inheriting /ModifiedNodeState /or /AbstractDocumentNodeState /(see class inheritance diagram) */AbstractDocumentNodeState#equals/ is checking against instances of /ModifiedNodeState /and /AbstractDocumentNodeState /but not /ModifiedDocumentNodeState/. * /AbstractDocumentNodeState#equals/ triggers the "slow" /AbstractNodeState#equals/ as a last resort We definitely do not see the full picture yet but guessing from a high-level perspective it looks like the introduction of /ModifiedDocumentNodeState /broke the equals logic in class /AbstractDocumentNodeState/. Would you agree with that conclusion? Is there anything further I can provide? _/Stacktrace:/_ MongoDocumentStore.query(Collection, String, String, String, long, int) line: 609 MongoDocumentStore.query(Collection, String, String, int) line: 598 DocumentNodeStore.readChildDocs(String, String, int) line: 129
Re: Slow performance in AbstractNodeState.equals
Hi Alexander Thanks for the detailed report and analysis. May I suggest that you create a ticket at https://issues.apache.org/jira/projects/OAK/issues with component documentmk providing all the details and attaching the patch? I think that's the best way to move forward. Unfortunately, I am not sufficiently familiar with that area of Oak to review the proposed modification, but Marcel Reutegger and Julian Reschke will for sure be able to provide you with feedback and suggestions and I would suggest you ping them directly in the Jira ticket. Kind regards Angela From: Alexander Lüders Sent: Friday, July 24, 2020 3:19 PM To: oak-dev@jackrabbit.apache.org Subject: Re: Slow performance in AbstractNodeState.equals Hi again, digging deeper I derived following patch which solved the performance issue: Index: oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 === --- oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (revision 17ea60b0bfb0ca3670c99208170a774a6c99fdfc) +++ oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (date 1595575140063) @@ -83,7 +83,11 @@ // revision does not match: might still be equals } else if (that instanceof ModifiedNodeState) { ModifiedNodeState modified = (ModifiedNodeState) that; -if (modified.getBaseState() == this) { +NodeState baseState = modified.getBaseState(); +if (baseState instanceof ModifiedDocumentNodeState) { +baseState = ((ModifiedDocumentNodeState) baseState).getBaseState(); +} +if (baseState == this) { return EqualsDiff.equals(this, modified); } } I am not entirely sure that this is correct but comparing with the ModifiedDocumentNodeState#equals method, I assume it to be. The latter method compares the passed object with its baseState not the ModifiedDocumentNodeState instance itself. This is the prime difference when comparing the differences of ModifiedDocumentNodeState#equals and AbstractNodeDocumentState#equals. The patch above transfers that logic to AbstractNodeDocumentState#equals. What do you think? Many thanks for your help. Best regards, Alexander Lüders Am 23.07.2020 um 17:26 schrieb Alexander Lüders: Dear Jackrabbit Oak developers, we are currently trying to hunt down a performance issue we are facing at a customer site. We strongly need your help on this so any comments would be greatly appreciated. We have a MongoDB as a backend of a Oak repository where we see hundreds of expensive queries being executed during a rather simple operation. It happens when we are saving a session in which we added a child node to a node already containing a very large number of children. This has not been the case with Jackrabbit Oak 1.4.8 and 1.4.26 but it is an issue with 1.10.8 and 1.22.3. Newer versions have not been tested yet but looking at the source we believe that they will show that performance issue too. What we know so far: * The queries against the MongoDocumentStore are triggered via AbstractNodeState.equals (see stacktrace below) * A TODO "inefficient unless there are very few child nodes" in that method indicates poor performance but this has been there since a long time and cannot explain the issue * OAK-7401 introduced the class ModifiedDocumentNodeState class which is not inheriting ModifiedNodeState or AbstractDocumentNodeState (see class inheritance diagram) * AbstractDocumentNodeState#equals is checking against instances of ModifiedNodeState and AbstractDocumentNodeState but not ModifiedDocumentNodeState. * AbstractDocumentNodeState#equals triggers the "slow" AbstractNodeState#equals as a last resort We definitely do not see the full picture yet but guessing from a high-level perspective it looks like the introduction of ModifiedDocumentNodeState broke the equals logic in class AbstractDocumentNodeState. Would you agree with that conclusion? Is there anything further I can provide? Stacktrace: MongoDocumentStore.query(Collection, String, String, String, long, int) line: 609 MongoDocumentStore.query(Collection, String, String, int) line: 598 DocumentNodeStore.readChildDocs(String, String, int) line: 1293 DocumentNodeStore.readChildren(AbstractDocumentNodeState, String, int) line: 1233 DocumentNodeStore$7.call() line: 1185 DocumentNodeStore$7.call() line: 1182 LocalCache$LocalManualCache$1.load(Object) line: 4724 LocalCache$LoadingValueReference.loadFuture(K, CacheLoader) line: 3522 LocalCache$Segment.loadSync(K, int, LoadingValueReference, CacheLoader) line: 2315 LocalCac
Re: Slow performance in AbstractNodeState.equals
Hi again, digging deeper I derived following patch which solved the performance issue: Index: oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 === --- oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (revision 17ea60b0bfb0ca3670c99208170a774a6c99fdfc) +++ oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/AbstractDocumentNodeState.java (date 1595575140063) @@ -83,7 +83,11 @@ // revision does not match: might still be equals } else if (that instanceof ModifiedNodeState) { ModifiedNodeState modified = (ModifiedNodeState) that; - if (modified.getBaseState() == this) { + NodeState baseState = modified.getBaseState(); + if (baseState instanceof ModifiedDocumentNodeState) { + baseState = ((ModifiedDocumentNodeState) baseState).getBaseState(); + } + if (baseState == this) { return EqualsDiff.equals(this, modified); } } I am not entirely sure that this is correct but comparing with the /ModifiedDocumentNodeState#equals/ method, I assume it to be. The latter method compares the passed object with its baseState not the /ModifiedDocumentNodeState/ instance itself. This is the prime difference when comparing the differences of /ModifiedDocumentNodeState#equals/ and /AbstractNodeDocumentState#equals/. The patch above transfers that logic to /AbstractNodeDocumentState#equals/. What do you think? Many thanks for your help. Best regards, Alexander Lüders Am 23.07.2020 um 17:26 schrieb Alexander Lüders: Dear Jackrabbit Oak developers, we are currently trying to hunt down a performance issue we are facing at a customer site. We strongly need your help on this so any comments would be greatly appreciated. We have a MongoDB as a backend of a Oak repository where we see hundreds of expensive queries being executed during a rather simple operation. It happens when we are saving a session in which we added a child node to a node already containing a very large number of children. This has not been the case with Jackrabbit Oak 1.4.8 and 1.4.26 but it is an issue with 1.10.8 and 1.22.3. Newer versions have not been tested yet but looking at the source we believe that they will show that performance issue too. What we know so far: * The queries against the /MongoDocumentStore /are triggered via /AbstractNodeState.equals/ (see stacktrace below) * A TODO "inefficient unless there are very few child nodes" in that method indicates poor performance but this has been there since a long time and cannot explain the issue * OAK-7401 introduced the class /ModifiedDocumentNodeState /class which is not inheriting /ModifiedNodeState /or /AbstractDocumentNodeState /(see class inheritance diagram) */AbstractDocumentNodeState#equals/ is checking against instances of /ModifiedNodeState /and /AbstractDocumentNodeState /but not /ModifiedDocumentNodeState/. * /AbstractDocumentNodeState#equals/ triggers the "slow" /AbstractNodeState#equals/ as a last resort We definitely do not see the full picture yet but guessing from a high-level perspective it looks like the introduction of /ModifiedDocumentNodeState /broke the equals logic in class /AbstractDocumentNodeState/. Would you agree with that conclusion? Is there anything further I can provide? _/Stacktrace:/_ MongoDocumentStore.query(Collection, String, String, String, long, int) line: 609 MongoDocumentStore.query(Collection, String, String, int) line: 598 DocumentNodeStore.readChildDocs(String, String, int) line: 1293 DocumentNodeStore.readChildren(AbstractDocumentNodeState, String, int) line: 1233 DocumentNodeStore$7.call() line: 1185 DocumentNodeStore$7.call() line: 1182 LocalCache$LocalManualCache$1.load(Object) line: 4724 LocalCache$LoadingValueReference.loadFuture(K, CacheLoadersuper K,V>) line: 3522 LocalCache$Segment.loadSync(K, int, LoadingValueReference, CacheLoader) line: 2315 LocalCache$Segment.lockedGetOrLoad(K, int, CacheLoaderK,V>) line: 2278 LocalCache$Segment.get(K, int, CacheLoader) line: 2193 LocalCache.get(K, CacheLoader) line: 3932 LocalCache$LocalManualCache.get(K, Callable) line: 4721 DocumentNodeStore.getChildren(AbstractDocumentNodeState, String, int) line: 1182 DocumentNodeState.getChildNodeCount(long) line: 293 AbstractNodeState.equals(NodeState, NodeState) line: 375 DocumentNodeState(AbstractDocumentNodeState).equals(Object) line: 91 AbstractNodeState.equals(NodeState, NodeState) line: 384 DocumentNodeState(AbstractDocumentNodeState).equals(Object) line: 91 DocumentNodeStoreBranch$InMemory.setRoot(NodeState) line: 481 DocumentNodeStoreBranch.setRoot(NodeState) line: 111