Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Htmlunit tuning #715

Merged
merged 19 commits into from
Jan 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e1b1090
Globally shared caches have to be thread-safe, if it is static, it is
rschwietzke Dec 21, 2023
36ae9a8
Only keep a hashmap for a namespace if we need it
rschwietzke Jan 12, 2024
faba021
ConcurrentHashmap is more efficent than a classic synchronized map
rschwietzke Jan 14, 2024
8dede64
Synchronization for changing a reference on a clone that is only know…
rschwietzke Jan 14, 2024
a788436
The clone might want to have the same synchronized list as the source
rschwietzke Jan 14, 2024
35f0af4
Attempt to use a CopyOnWriteArrayList instead of two lists and sychro…
rschwietzke Jan 15, 2024
aa1474b
Revert "Attempt to use a CopyOnWriteArrayList instead of two lists an…
rschwietzke Jan 15, 2024
d925d17
Save references, a dedicated lock and a second holder object, listene…
rschwietzke Jan 15, 2024
c849ab4
Different concept for an ordered map that avoid the need for an extra…
rschwietzke Jan 22, 2024
ff32528
JavaScript mimetype lookup via FashHashMap and upper and lowercasing …
rschwietzke Jan 22, 2024
63984e3
Removed lowercase caching. Memory access for a large random map, espe…
rschwietzke Jan 22, 2024
2ef72c1
Just run a cheap check first before comparing stuff
rschwietzke Jan 23, 2024
5978331
Using OrderHashMap instead of LinkedHashMap plus List
rschwietzke Jan 23, 2024
ed6f4c7
List should be cheaper than a map for the expected data amount even w…
rschwietzke Jan 23, 2024
8dc6909
Lookup of elements is more efficient now
rschwietzke Jan 23, 2024
d1e7ccd
Added performance idea notepad
rschwietzke Jan 23, 2024
70454f0
Fixed merge issue
rschwietzke Jan 23, 2024
84d4c24
Fixing ordered map growth and assuring it is not stuck when looking u…
rschwietzke Jan 25, 2024
9208d7b
Styleguide updates
rschwietzke Jan 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions PERFORMANCE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
= Ideas for Performance Changes

* DomNode: Use Copy on Write for listeners to safe on object holders, but CopyOnWriteArrayList is not good enough
* DomNode: Remove synchronized and find something better without extra cost for more objects, maybe a ReentrantLock at the SGMLPage

3 changes: 1 addition & 2 deletions src/main/java/org/htmlunit/DefaultPageCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,11 @@ public static PageType determinePageType(final String contentType) {
}

final String contentTypeLC = org.htmlunit.util.StringUtils
.toRootLowerCaseWithCache(contentType);
.toRootLowerCase(contentType);

if (MimeType.isJavascriptMimeType(contentTypeLC)) {
return PageType.JAVASCRIPT;
}

switch (contentTypeLC) {
case MimeType.TEXT_HTML:
case "image/svg+xml":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ public String getStyleAttribute(final String name) {
final String value = element.getValue();
if (!"content".equals(name)
&& !value.contains("url")) {
return org.htmlunit.util.StringUtils.toRootLowerCaseWithCache(value);
return org.htmlunit.util.StringUtils.toRootLowerCase(value);
}
return value;
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/htmlunit/css/CssStyleSheet.java
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,8 @@ public static boolean selects(final BrowserVersion browserVersion,
final String a = element.getAttribute(beginHyphenAttributeCondition.getLocalName());
if (beginHyphenAttributeCondition.isCaseInSensitive()) {
return selectsHyphenSeparated(
org.htmlunit.util.StringUtils.toRootLowerCaseWithCache(v),
org.htmlunit.util.StringUtils.toRootLowerCaseWithCache(a));
org.htmlunit.util.StringUtils.toRootLowerCase(v),
org.htmlunit.util.StringUtils.toRootLowerCase(a));
}
return selectsHyphenSeparated(v, a);

Expand All @@ -636,8 +636,8 @@ public static boolean selects(final BrowserVersion browserVersion,
final String a2 = element.getAttribute(oneOfAttributeCondition.getLocalName());
if (oneOfAttributeCondition.isCaseInSensitive()) {
return selectsOneOf(
org.htmlunit.util.StringUtils.toRootLowerCaseWithCache(v2),
org.htmlunit.util.StringUtils.toRootLowerCaseWithCache(a2));
org.htmlunit.util.StringUtils.toRootLowerCase(v2),
org.htmlunit.util.StringUtils.toRootLowerCase(a2));
}
return selectsOneOf(v2, a2);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public String getStyleAttribute(final String name) {
if (element != null && element.getValue() != null) {
final String value = element.getValue();
if (!value.contains("url")) {
return StringUtils.toRootLowerCaseWithCache(value);
return StringUtils.toRootLowerCase(value);
}
return value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/htmlunit/html/BaseFrameElement.java
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public final void setSrcAttribute(final String attribute) {
@Override
protected void setAttributeNS(final String namespaceURI, final String qualifiedName, String attributeValue,
final boolean notifyAttributeChangeListeners, final boolean notifyMutationObserver) {
final String qualifiedNameLC = org.htmlunit.util.StringUtils.toRootLowerCaseWithCache(qualifiedName);
final String qualifiedNameLC = org.htmlunit.util.StringUtils.toRootLowerCase(qualifiedName);
if (null != attributeValue && SRC_ATTRIBUTE.equals(qualifiedNameLC)) {
attributeValue = attributeValue.trim();
}
Expand Down
14 changes: 8 additions & 6 deletions src/main/java/org/htmlunit/html/DefaultElementFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand All @@ -28,6 +27,7 @@
import org.htmlunit.BrowserVersion;
import org.htmlunit.SgmlPage;
import org.htmlunit.javascript.configuration.JavaScriptConfiguration;
import org.htmlunit.util.OrderedFastHashMap;
import org.xml.sax.Attributes;

/**
Expand Down Expand Up @@ -779,23 +779,25 @@ public HtmlElement createElementNS(final SgmlPage page, final String namespaceUR
* @return the map of attribute values for {@link HtmlElement}s
*/
static Map<String, DomAttr> toMap(final SgmlPage page, final Attributes attributes) {
if (attributes == null) {
return null;
}
final int length = attributes == null ? 0 : attributes.getLength();
final Map<String, DomAttr> attributeMap = new OrderedFastHashMap<>(length);

final Map<String, DomAttr> attributeMap = new LinkedHashMap<>(attributes.getLength());
for (int i = 0; i < attributes.getLength(); i++) {
for (int i = 0; i < length; i++) {
final String qName = attributes.getQName(i);

// browsers consider only first attribute (ex: <div id='foo' id='something'>...</div>)
if (!attributeMap.containsKey(qName)) {
String namespaceURI = attributes.getURI(i);

if (namespaceURI != null && namespaceURI.isEmpty()) {
namespaceURI = null;
}

final DomAttr newAttr = new DomAttr(page, namespaceURI, qName, attributes.getValue(i), true);
attributeMap.put(qName, newAttr);
}
}

return attributeMap;
}

Expand Down
57 changes: 38 additions & 19 deletions src/main/java/org/htmlunit/html/DomElement.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -52,13 +51,15 @@
import org.htmlunit.cssparser.parser.selector.Selector;
import org.htmlunit.cssparser.parser.selector.SelectorList;
import org.htmlunit.cssparser.parser.selector.SelectorSpecificity;
import org.htmlunit.cyberneko.util.FastHashMap;
import org.htmlunit.javascript.AbstractJavaScriptEngine;
import org.htmlunit.javascript.HtmlUnitContextFactory;
import org.htmlunit.javascript.JavaScriptEngine;
import org.htmlunit.javascript.host.event.Event;
import org.htmlunit.javascript.host.event.EventTarget;
import org.htmlunit.javascript.host.event.MouseEvent;
import org.htmlunit.javascript.host.event.PointerEvent;
import org.htmlunit.util.OrderedFastHashMap;
import org.htmlunit.util.StringUtils;
import org.w3c.dom.Attr;
import org.w3c.dom.DOMException;
Expand Down Expand Up @@ -104,7 +105,7 @@ public class DomElement extends DomNamespaceNode implements Element {
private NamedAttrNodeMapImpl attributes_;

/** The map holding the namespaces, keyed by URI. */
private final Map<String, String> namespaces_ = new HashMap<>();
private FastHashMap<String, String> namespaces_;

/** Cache for the styles. */
private String styleString_;
Expand All @@ -128,13 +129,18 @@ public DomElement(final String namespaceURI, final String qualifiedName, final S
final Map<String, DomAttr> attributes) {
super(namespaceURI, qualifiedName, page);

if (attributes != null && !attributes.isEmpty()) {
if (attributes != null) {
attributes_ = new NamedAttrNodeMapImpl(this, isAttributeCaseSensitive(), attributes);
for (final DomAttr entry : attributes_.values()) {

for (final DomAttr entry : attributes.values()) {
entry.setParentNode(this);
final String attrNamespaceURI = entry.getNamespaceURI();
final String prefix = entry.getPrefix();

if (attrNamespaceURI != null && prefix != null) {
if (namespaces_ == null) {
namespaces_ = new FastHashMap<>(1, 0.5f);
}
namespaces_.put(attrNamespaceURI, prefix);
}
}
Expand Down Expand Up @@ -384,7 +390,7 @@ String getQualifiedName(final String namespaceURI, final String localName) {
qualifiedName = localName;
}
else {
final String prefix = namespaces_.get(namespaceURI);
final String prefix = namespaces_ == null ? null : namespaces_.get(namespaceURI);
if (prefix == null) {
qualifiedName = null;
}
Expand Down Expand Up @@ -532,6 +538,9 @@ protected void setAttributeNS(final String namespaceURI, final String qualifiedN
attributes_.put(qualifiedName, newAttr);

if (namespaceURI != null) {
if (namespaces_ == null) {
namespaces_ = new FastHashMap<>(1, 0.5f);
}
namespaces_.put(namespaceURI, newAttr.getPrefix());
}
}
Expand Down Expand Up @@ -1656,16 +1665,15 @@ class NamedAttrNodeMapImpl implements Map<String, DomAttr>, NamedNodeMap, Serial
private static final DomAttr[] EMPTY_ARRAY = new DomAttr[0];
protected static final NamedAttrNodeMapImpl EMPTY_MAP = new NamedAttrNodeMapImpl();

private final Map<String, DomAttr> map_ = new LinkedHashMap<>();
private boolean dirty_;
private DomAttr[] attrPositions_ = EMPTY_ARRAY;
private final OrderedFastHashMap<String, DomAttr> map_;
private final DomElement domNode_;
private final boolean caseSensitive_;

private NamedAttrNodeMapImpl() {
super();
domNode_ = null;
caseSensitive_ = true;
map_ = new OrderedFastHashMap<>(0);
}

NamedAttrNodeMapImpl(final DomElement domNode, final boolean caseSensitive) {
Expand All @@ -1675,12 +1683,30 @@ private NamedAttrNodeMapImpl() {
}
domNode_ = domNode;
caseSensitive_ = caseSensitive;
map_ = new OrderedFastHashMap<>(0);
}

NamedAttrNodeMapImpl(final DomElement domNode, final boolean caseSensitive,
final Map<String, DomAttr> attributes) {
this(domNode, caseSensitive);
putAll(attributes);
super();
if (domNode == null) {
throw new IllegalArgumentException("Provided domNode can't be null.");
}
domNode_ = domNode;
caseSensitive_ = caseSensitive;

// we expect a special map here, if we don't get it... we have to create us one
if (caseSensitive && attributes instanceof OrderedFastHashMap) {
// no need to rework the map at all, we are case sensitive, so
// we keep all attributes and we got the right map from outside too
map_ = (OrderedFastHashMap) attributes;
}
else {
// this is more expensive but atypical, so we don't have to care that much
map_ = new OrderedFastHashMap<>(attributes.size());
// this will create a new map with all case lowercased and
putAll(attributes);
}
}

/**
Expand All @@ -1703,7 +1729,7 @@ private String fixName(final String name) {
if (caseSensitive_) {
return name;
}
return StringUtils.toRootLowerCaseWithCache(name);
return StringUtils.toRootLowerCase(name);
}

/**
Expand All @@ -1725,11 +1751,7 @@ public Node item(final int index) {
if (index < 0 || index >= map_.size()) {
return null;
}
if (dirty_) {
attrPositions_ = map_.values().toArray(attrPositions_);
dirty_ = false;
}
return attrPositions_[index];
return map_.getValue(index);
}

/**
Expand Down Expand Up @@ -1773,7 +1795,6 @@ public Node setNamedItemNS(final Node node) throws DOMException {
@Override
public DomAttr put(final String key, final DomAttr value) {
final String name = fixName(key);
dirty_ = true;
return map_.put(name, value);
}

Expand All @@ -1784,7 +1805,6 @@ public DomAttr put(final String key, final DomAttr value) {
public DomAttr remove(final Object key) {
if (key instanceof String) {
final String name = fixName((String) key);
dirty_ = true;
return map_.remove(name);
}
return null;
Expand All @@ -1795,7 +1815,6 @@ public DomAttr remove(final Object key) {
*/
@Override
public void clear() {
dirty_ = true;
map_.clear();
}

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/org/htmlunit/html/DomNamespaceNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@
*/
package org.htmlunit.html;

import java.util.Locale;

import org.htmlunit.SgmlPage;
import org.htmlunit.WebAssert;
import org.htmlunit.html.xpath.XPathHelper;
import org.htmlunit.javascript.host.dom.Document;
import org.htmlunit.util.StringUtils;

/**
* Intermediate base class for DOM Nodes that have namespaces. That includes HtmlElement and HtmlAttr.
Expand Down Expand Up @@ -61,7 +60,7 @@ protected DomNamespaceNode(final String namespaceURI, final String qualifiedName
prefix_ = qualifiedName_.substring(0, colonPosition);
}

localNameLC_ = localName_.toLowerCase(Locale.ROOT);
localNameLC_ = StringUtils.toRootLowerCase(localName_);
}

/**
Expand Down
Loading
Loading