You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "nfsantos (via GitHub)" <gi...@apache.org> on 2024/07/30 13:51:40 UTC
[PR] OAK-10989 - Performance improvements to JSON parsing [jackrabbit-oak]
nfsantos opened a new pull request, #1612:
URL: https://github.com/apache/jackrabbit-oak/pull/1612
In JsonDeserializer:
- when reading a property, check for the most common types first (String).
- make instances reusable by creating a new MemoryNodeBuilder for every call to deserialize
- Remove usages of Guava collections.
- Move the methods in the inner class DeserializationSupport to top level methods in JsonDeserializer, as these inner class was not accessed anywhere outside JsonDeserializer and did not contain any state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10989 - Performance improvements to JSON parsing [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1612:
URL: https://github.com/apache/jackrabbit-oak/pull/1612#discussion_r1701428350
##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonDeserializer.java:
##########
@@ -39,49 +39,67 @@
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
-import static org.apache.jackrabbit.guava.common.collect.ImmutableSet.of;
-import static java.util.Collections.emptyList;
import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
import static org.apache.jackrabbit.JcrConstants.NT_UNSTRUCTURED;
import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty;
public class JsonDeserializer {
- public static final String OAK_CHILD_ORDER = ":childOrder";
- private final BlobDeserializer blobHandler;
- private final NodeBuilder builder;
- private final DeserializationSupport deserializationSupport;
+ /**
+ * Provides support for inferring types for some common property name and types
+ */
+ private static final Set<String> NAME_PROPS = Set.of(JCR_PRIMARYTYPE, JCR_MIXINTYPES);
+ private static final String ORDERABLE_TYPE = NT_UNSTRUCTURED;
+ private static final String OAK_CHILD_ORDER = ":childOrder";
- private JsonDeserializer(BlobDeserializer blobHandler, NodeBuilder builder, DeserializationSupport support) {
- this.blobHandler = blobHandler;
- this.builder = builder;
- this.deserializationSupport = support;
+ private static Type<?> inferPropertyType(String propertyName, String jsonString) {
+ if (NAME_PROPS.contains(propertyName) && hasSingleColon(jsonString)) {
+ return Type.NAME;
+ }
+ return Type.UNDEFINED;
}
- public JsonDeserializer(BlobDeserializer blobHandler) {
- this(blobHandler, EMPTY_NODE.builder(), DeserializationSupport.INSTANCE);
+ private static boolean hasOrderableChildren(NodeBuilder builder) {
+ PropertyState primaryType = builder.getProperty(JCR_PRIMARYTYPE);
+ return primaryType != null && ORDERABLE_TYPE.equals(primaryType.getValue(Type.NAME));
+ }
+
+ private static boolean hasSingleColon(String jsonString) {
+ // In case the primaryType was encoded then it would be like
Review Comment:
Yes, I moved it from the bottom of the class to be at the top, as is the convention for static code.
Your proposed solution might indeed be faster, given that `indexOf()` is intrinsified, so it is executed with native code, likely using vector instructions. This should be faster than a loop on `charAt()`. Might be worth to explore, although the gains might not be large because property names are usually small.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10989 - Performance improvements to JSON parsing [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller commented on code in PR #1612:
URL: https://github.com/apache/jackrabbit-oak/pull/1612#discussion_r1701411719
##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonDeserializer.java:
##########
@@ -39,49 +39,67 @@
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
-import static org.apache.jackrabbit.guava.common.collect.ImmutableSet.of;
-import static java.util.Collections.emptyList;
import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
import static org.apache.jackrabbit.JcrConstants.NT_UNSTRUCTURED;
import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty;
public class JsonDeserializer {
- public static final String OAK_CHILD_ORDER = ":childOrder";
- private final BlobDeserializer blobHandler;
- private final NodeBuilder builder;
- private final DeserializationSupport deserializationSupport;
+ /**
+ * Provides support for inferring types for some common property name and types
+ */
+ private static final Set<String> NAME_PROPS = Set.of(JCR_PRIMARYTYPE, JCR_MIXINTYPES);
+ private static final String ORDERABLE_TYPE = NT_UNSTRUCTURED;
+ private static final String OAK_CHILD_ORDER = ":childOrder";
- private JsonDeserializer(BlobDeserializer blobHandler, NodeBuilder builder, DeserializationSupport support) {
- this.blobHandler = blobHandler;
- this.builder = builder;
- this.deserializationSupport = support;
+ private static Type<?> inferPropertyType(String propertyName, String jsonString) {
+ if (NAME_PROPS.contains(propertyName) && hasSingleColon(jsonString)) {
+ return Type.NAME;
+ }
+ return Type.UNDEFINED;
}
- public JsonDeserializer(BlobDeserializer blobHandler) {
- this(blobHandler, EMPTY_NODE.builder(), DeserializationSupport.INSTANCE);
+ private static boolean hasOrderableChildren(NodeBuilder builder) {
+ PropertyState primaryType = builder.getProperty(JCR_PRIMARYTYPE);
+ return primaryType != null && ORDERABLE_TYPE.equals(primaryType.getValue(Type.NAME));
+ }
+
+ private static boolean hasSingleColon(String jsonString) {
+ // In case the primaryType was encoded then it would be like
Review Comment:
What about using two indexOf operations instead? E.g. one to get the first colon, another to get the last, and then check if they are equal. I think that would be simpler and faster. Ah I see it's just code that is moved around...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10989 - Performance improvements to JSON parsing [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos merged PR #1612:
URL: https://github.com/apache/jackrabbit-oak/pull/1612
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org