Skip to content

Commit

Permalink
#340 optimize CPU and memory consumption in ClassCondition
Browse files Browse the repository at this point in the history
My profiler shows that some remarkable % of CPU is spent on generating this string: (" " + c + " "). Now we check for CSS class without generating the new string. The code is longer, but faster.
  • Loading branch information
asolntsev committed Jul 19, 2024
1 parent 90d4d8e commit 5bc115c
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,13 @@ protected boolean compare(String attrValue, String conditionValue) {
}
}

private static class ClassCondition extends Condition {
private final String _paddedClassName;
final static class ClassCondition extends Condition {
private final String className;
private final int classNameLength;

private ClassCondition(String className) {
_paddedClassName = " " + className + " ";
this.className = className;
this.classNameLength = className.length();
}

@Override
Expand All @@ -284,14 +286,27 @@ boolean matches(Node e, AttributeResolver attRes, TreeResolver treeRes) {
return false;
}
String c = attRes.getClass(e);
if (c == null) {
return false;
}
return c != null && containsClassName(c);
}

boolean containsClassName(String classAttribute) {
return containsClassName(classAttribute, -1);
}

private boolean containsClassName(String classAttribute, int fromIndex) {
// This is much faster than calling `split()` and comparing individual values in a loop.
// NOTE: In jQuery, for example, the attribute value first has whitespace normalized to spaces. But
// in an XML DOM, space normalization in attributes is supposed to have happened already.
return (" " + c + " ").contains(_paddedClassName);
int index = classAttribute.indexOf(className, fromIndex);
if (index == -1) return false;

return isWhitespace(classAttribute, index - 1)
&& isWhitespace(classAttribute, index + classNameLength)
|| containsClassName(classAttribute, index + classNameLength);
}

private boolean isWhitespace(String classAttribute, int index) {
return index < 0 || index >= classAttribute.length() || Character.isWhitespace(classAttribute.charAt(index));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package org.xhtmlrenderer.css.newmatch;

import org.junit.jupiter.api.Test;
import org.xhtmlrenderer.css.newmatch.Condition.ClassCondition;

import static org.assertj.core.api.Assertions.assertThat;
import static org.xhtmlrenderer.css.newmatch.Condition.createClassCondition;

class ClassConditionTest {
private final ClassCondition condition = (ClassCondition) createClassCondition("active");

@Test
void classAttributeEqualsExpectedClassName() {
assertThat(condition.containsClassName("active")).isTrue();
assertThat(condition.containsClassName("activex")).isFalse();
}

@Test
void classAttributeContainsExpectedClassName() {
assertThat(condition.containsClassName("foo active bar")).isTrue();
assertThat(condition.containsClassName("foo active ")).isTrue();
assertThat(condition.containsClassName(" active bar")).isTrue();
assertThat(condition.containsClassName(" active ")).isTrue();
}

@Test
void classAttributeStartsWithExpectedClassName() {
assertThat(condition.containsClassName("active foo bar")).isTrue();
assertThat(condition.containsClassName(" active foo bar")).isTrue();
assertThat(condition.containsClassName("activex foo bar")).isFalse();
assertThat(condition.containsClassName("inactive foo bar")).isFalse();
}

@Test
void classAttributeEndsWithExpectedClassName() {
assertThat(condition.containsClassName("foo bar active")).isTrue();
assertThat(condition.containsClassName("foo bar active ")).isTrue();
assertThat(condition.containsClassName("foo bar inactive")).isFalse();
assertThat(condition.containsClassName("foo bar activeactive")).isFalse();
assertThat(condition.containsClassName("foo bar active-active")).isFalse();
assertThat(condition.containsClassName("foo bar active_active")).isFalse();
assertThat(condition.containsClassName("foo bar activex")).isFalse();
}

@Test
void classAttributeContainsSimilarClassNames() {
assertThat(condition.containsClassName("activex active inactive")).isTrue();
assertThat(condition.containsClassName("activex _active inactive")).isFalse();
}

@Test
void classNotMatches() {
assertThat(condition.containsClassName("")).isFalse();
assertThat(condition.containsClassName("_active")).isFalse();
assertThat(condition.containsClassName("active_")).isFalse();
assertThat(condition.containsClassName("act ive")).isFalse();
}
}

0 comments on commit 5bc115c

Please sign in to comment.