Skip to content

Commit

Permalink
perf: Use trusted constructors when creating nodes from parser.
Browse files Browse the repository at this point in the history
This commit provides a few memory optimizations that relies on fact that
we trust the internal code that invokes `NodesFactory.createLogicalNode`
and `NodesFactory.createComparisonNode`.
  • Loading branch information
nstdio committed Dec 11, 2024
1 parent 6dce529 commit 46fcee9
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 5 deletions.
90 changes: 90 additions & 0 deletions src/main/java/cz/jirutka/rsql/parser/NodesFactoryAccess.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package cz.jirutka.rsql.parser;

import static java.util.logging.Level.WARNING;

import cz.jirutka.rsql.parser.ast.ComparisonNode;
import cz.jirutka.rsql.parser.ast.LogicalNode;
import cz.jirutka.rsql.parser.ast.LogicalOperator;
import cz.jirutka.rsql.parser.ast.Node;
import cz.jirutka.rsql.parser.ast.NodesFactory;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodHandles.Lookup;
import java.lang.reflect.Method;
import java.util.List;
import java.util.logging.Logger;

final class NodesFactoryAccess {

private NodesFactoryAccess() {
}

private static final MethodHandle LOGICAL_NODE_MH;
private static final MethodHandle COMP_NODE_MH;

static {
Lookup lookup = MethodHandles.lookup();

LOGICAL_NODE_MH = methodHandle(lookup, "logicalNodeTrusted", LogicalOperator.class,
List.class);
COMP_NODE_MH = methodHandle(lookup, "comparisonNodeTrusted", String.class, String.class,
List.class);
}

private static MethodHandle methodHandle(Lookup lookup, String name, Class<?>... parameterTypes) {
Method m = null;
MethodHandle mh = null;

try {
m = NodesFactory.class.getDeclaredMethod(name, parameterTypes);
m.setAccessible(true);

mh = lookup.unreflect(m);
} catch (Throwable e) {
logger().log(WARNING, "Unable to initialize MethodHandle for {0}", new Object[]{name, e});
} finally {
if (m != null) {
m.setAccessible(false);
}
}

return mh;
}

static LogicalNode create(NodesFactory factory, LogicalOperator operator, List<Node> children) {
if (LOGICAL_NODE_MH == null) {
return factory.createLogicalNode(operator, children);
} else {
try {
return (LogicalNode) LOGICAL_NODE_MH.invoke(factory, operator, children);
} catch (RuntimeException e) {
throw e;
} catch (Throwable e) {
logger().log(WARNING, "The logicalNodeTrusted unexpectedly thrown exception", e);

return factory.createLogicalNode(operator, children);
}
}
}

static ComparisonNode create(NodesFactory factory, String operatorToken, String selector, List<String> arguments)
throws UnknownOperatorException {
if (COMP_NODE_MH == null) {
return factory.createComparisonNode(operatorToken, selector, arguments);
} else {
try {
return (ComparisonNode) COMP_NODE_MH.invoke(factory, operatorToken, selector, arguments);
} catch (RuntimeException | UnknownOperatorException e) {
throw e;
} catch (Throwable e) {
logger().log(WARNING, "The comparisonNodeTrusted unexpectedly thrown exception", e);

return factory.createComparisonNode(operatorToken, selector, arguments);
}
}
}

private static Logger logger() {
return Logger.getLogger(NodesFactoryAccess.class.getName());
}
}
4 changes: 4 additions & 0 deletions src/main/java/cz/jirutka/rsql/parser/ast/AndNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public AndNode(List<? extends Node> children) {
super(LogicalOperator.AND, children);
}

AndNode(List<? extends Node> children, boolean trusted) {
super(LogicalOperator.AND, children, trusted);
}

public AndNode withChildren(List<? extends Node> children) {
return new AndNode(children);
}
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/cz/jirutka/rsql/parser/ast/LogicalNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,17 @@ public abstract class LogicalNode extends AbstractNode implements Iterable<Node>
* @param children Children nodes, i.e. operands; must not be <tt>null</tt>.
*/
protected LogicalNode(LogicalOperator operator, List<? extends Node> children) {
this(operator, new ArrayList<>(children), true);
}

LogicalNode(LogicalOperator operator, List<? extends Node> children, @SuppressWarnings("unused") boolean trusted) {
Assert.notNull(operator, "operator must not be null");
Assert.notNull(children, "children must not be null");

this.operator = operator;
this.children = unmodifiableList(new ArrayList<>(children));
this.children = unmodifiableList(children);
}


/**
* Returns a copy of this node with the specified children nodes.
*
Expand Down
33 changes: 33 additions & 0 deletions src/main/java/cz/jirutka/rsql/parser/ast/NodesFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,37 @@ public ComparisonNode createComparisonNode(
throw new UnknownOperatorException(operatorToken);
}
}

/**
* Invoked from {@code cz.jirutka.rsql.parser.NodesFactoryAccess#create(NodesFactory, LogicalOperator, List)} via
* method handle.
*/
@SuppressWarnings("unused")
LogicalNode logicalNodeTrusted(LogicalOperator operator, List<Node> children) {
switch (operator) {
case AND:
return new AndNode(children, true);
case OR:
return new OrNode(children, true);

// this normally can't happen
default:
throw new IllegalStateException("Unknown operator: " + operator);
}
}

/**
* Invoked from {@code cz.jirutka.rsql.parser.NodesFactoryAccess#create(NodesFactory, String, String, List)} via
* method handle.
*/
@SuppressWarnings("unused")
ComparisonNode comparisonNodeTrusted(String operatorToken, String selector, List<String> arguments)
throws UnknownOperatorException {
ComparisonOperator op = comparisonOperators.get(operatorToken);
if (op != null) {
return new ComparisonNode(op, selector, arguments, true);
} else {
throw new UnknownOperatorException(operatorToken);
}
}
}
4 changes: 4 additions & 0 deletions src/main/java/cz/jirutka/rsql/parser/ast/OrNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public OrNode(List<? extends Node> children) {
super(LogicalOperator.OR, children);
}

OrNode(List<? extends Node> children, boolean trusted) {
super(LogicalOperator.OR, children, trusted);
}

public OrNode withChildren(List<? extends Node> children) {
return new OrNode(children);
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/javacc/RSQLParser.jj
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ Node Or():
<OR> node = And() { nodes.add(node); }
)*
{
return nodes.size() != 1 ? factory.createLogicalNode(LogicalOperator.OR, nodes) : nodes.get(0);
return nodes.size() != 1 ? NodesFactoryAccess.create(factory, LogicalOperator.OR, nodes) : nodes.get(0);
}
}

Expand All @@ -144,7 +144,7 @@ Node And():
<AND> node = Constraint() { nodes.add(node); }
)*
{
return nodes.size() != 1 ? factory.createLogicalNode(LogicalOperator.AND, nodes) : nodes.get(0);
return nodes.size() != 1 ? NodesFactoryAccess.create(factory, LogicalOperator.AND, nodes) : nodes.get(0);
}
}

Expand Down Expand Up @@ -179,7 +179,7 @@ ComparisonNode Comparison():
{
( sel = Selector() op = Operator() args = Arguments() )
{
return factory.createComparisonNode(op, sel, args);
return NodesFactoryAccess.create(factory, op, sel, args);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package cz.jirutka.rsql.parser

import cz.jirutka.rsql.parser.ast.ComparisonNode
import cz.jirutka.rsql.parser.ast.LogicalOperator
import cz.jirutka.rsql.parser.ast.NodesFactory
import spock.lang.Specification

import static cz.jirutka.rsql.parser.ast.RSQLOperators.*

class NodesFactoryAccessSpec extends Specification {

def 'Should use trusted constructors for creating logical node'(LogicalOperator op) {
given:
def factory = new NodesFactory(defaultOperators())
def children = [
new ComparisonNode(EQUAL, 'a1', ['b1']),
new ComparisonNode(NOT_EQUAL, 'a2', ['b2'])
]

when:
def actual = NodesFactoryAccess.create(factory, op, children)

then:
actual.children.size() == children.size()

and: 'original is not modified by modifying copy'
def childrenCopy = actual.children
childrenCopy.removeLast()
childrenCopy.size() + 1 == children.size()

and: 'but when modifying original it must change in the LogicalNode'
children.removeLast()

actual.children.size() == children.size()

where:
op << LogicalOperator.values()
}

def 'Should use trusted constructors for creating comparison node'() {
given:
def factory = new NodesFactory(defaultOperators())
def arguments = ['x', 'y', 'z']

when:
def actual = NodesFactoryAccess.create(factory, '=in=', 'a1', arguments)

then:
actual.arguments.size() == arguments.size()

and: 'original is not modified by modifying copy'
def childrenCopy = actual.arguments
childrenCopy.removeLast()
childrenCopy.size() + 1 == arguments.size()

and: 'but when modifying original it must change in the LogicalNode'
arguments.removeLast()

actual.arguments.size() == arguments.size()
}
}

0 comments on commit 46fcee9

Please sign in to comment.