Skip to content

Commit

Permalink
8199216: Quadratic layout time with nested nodes and pseudo-class in …
Browse files Browse the repository at this point in the history
…style sheet

Reviewed-by: angorya, jvos, mstrauss, kcr, mhanl
  • Loading branch information
hjohn committed Sep 15, 2023
1 parent f185974 commit 5e145cc
Show file tree
Hide file tree
Showing 10 changed files with 236 additions and 131 deletions.
Expand Up @@ -181,10 +181,16 @@ public boolean remove(Object o) {
return false;
}

T t = cast(o);
Class<T> elementType = getElementType();

final int element = getIndex(t) / Long.SIZE;
final long bit = 1l << (getIndex(t) % Long.SIZE);
if (!elementType.isInstance(o)) { // if cast failed, it can't be part of this set, so not modified
return false;
}

T t = elementType.cast(o);
int index = getIndex(t);
int element = index / Long.SIZE;
long bit = 1l << (index % Long.SIZE);

if (element >= bits.length) {
// not in this Set!
Expand Down Expand Up @@ -219,18 +225,22 @@ public boolean contains(Object o) {
return false;
}

final T t = cast(o);
Class<T> elementType = getElementType();

final int element = getIndex(t) / Long.SIZE;
final long bit = 1l << (getIndex(t) % Long.SIZE);
if (!elementType.isInstance(o)) {
return false;
}

int index = getIndex(elementType.cast(o));
int element = index / Long.SIZE;
long bit = 1L << (index % Long.SIZE);

return (element < bits.length) && (bits[element] & bit) == bit;
}

/** {@inheritDoc} */
@Override
public boolean containsAll(Collection<?> c) {
if (this.getClass() != c.getClass()) {
if (this.getClass() != c.getClass()) { // implicit null check here is intended
for (Object obj : c) {
if (!contains(obj)) {
return false;
Expand Down Expand Up @@ -259,11 +269,9 @@ public boolean containsAll(Collection<?> c) {
return true;
}


/** {@inheritDoc} */
@Override
public boolean addAll(Collection<? extends T> c) {
if (this.getClass() != c.getClass()) {
if (this.getClass() != c.getClass()) { // implicit null check here is intended
boolean modified = false;

for (T obj : c) {
Expand Down Expand Up @@ -336,10 +344,9 @@ public boolean addAll(Collection<? extends T> c) {

}

/** {@inheritDoc} */
@Override
public boolean retainAll(Collection<?> c) {
if (this.getClass() != c.getClass()) {
if (this.getClass() != c.getClass()) { // implicit null check here is intended
boolean modified = false;

for (Iterator<T> iterator = this.iterator(); iterator.hasNext();) {
Expand Down Expand Up @@ -426,11 +433,9 @@ public boolean retainAll(Collection<?> c) {
return modified;
}


/** {@inheritDoc} */
@Override
public boolean removeAll(Collection<?> c) {
if (this.getClass() != c.getClass()) {
if (this.getClass() != c.getClass()) { // implicit null check here is intended
boolean modified = false;

for (Object obj : c) {
Expand Down Expand Up @@ -495,7 +500,6 @@ public boolean removeAll(Collection<?> c) {
return modified;
}

/** {@inheritDoc} */
@Override
public void clear() {

Expand Down Expand Up @@ -539,31 +543,31 @@ public boolean equals(Object obj) {
private boolean equalsBitSet(BitSet<?> other) {
int a = this.bits != null ? this.bits.length : 0;
int b = other.bits != null ? other.bits.length : 0;
int max = Math.max(a, b);

if (a != b) return false;
for (int i = 0; i < max; i++) {
long m0 = i >= a ? 0 : this.bits[i];
long m1 = i >= b ? 0 : other.bits[i];

for(int m=0; m<a; m++) {
final long m0 = this.bits[m];
final long m1 = other.bits[m];
if (m0 != m1) {
return false;
}
}

return true;
}

abstract protected T getT(int index);
abstract protected int getIndex(T t);
protected abstract T getT(int index);
protected abstract int getIndex(T t);

/*
* Try to cast the arg to a T.
* @throws ClassCastException if the class of the argument is
* is not a T
* @throws NullPointerException if the argument is null
/**
* Returns the element type.
*
* @return a {@link Class} of type {@code T}, never {@code null}
*/
abstract protected T cast(Object o);
protected abstract Class<T> getElementType();

protected long[] getBits() {
long[] getBits() {
return bits;
}

Expand Down
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package com.sun.javafx.css;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import javafx.css.PseudoClass;

/**
* A cache for immutable sets of {@link PseudoClass}es.
*/
public class ImmutablePseudoClassSetsCache {
private static final Map<Set<PseudoClass>, Set<PseudoClass>> CACHE = new HashMap<>();

/**
* Returns an immutable set of {@link PseudoClass}es.
* <p>
* Note: this method may or may not return the same instance for the same set of
* {@link PseudoClass}es.
*
* @param pseudoClasses a set of {@link PseudoClass} to make immutable, cannot be {@code null}
* @return an immutable set of {@link PseudoClass}es, never {@code null}
* @throws NullPointerException when {@code pseudoClasses} is {@code null} or contains {@code null}s
*/
public static Set<PseudoClass> of(Set<PseudoClass> pseudoClasses) {
Set<PseudoClass> cachedSet = CACHE.get(Objects.requireNonNull(pseudoClasses, "pseudoClasses cannot be null"));

if (cachedSet != null) {
return cachedSet;
}

Set<PseudoClass> copy = Set.copyOf(pseudoClasses);

CACHE.put(copy, copy);

return copy;
}
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -54,35 +54,6 @@ public PseudoClassState() {
}
}

/** {@inheritDoc} */
@Override
public Object[] toArray() {
return toArray(new PseudoClass[size()]);
}

/** {@inheritDoc} */
@Override
public <T> T[] toArray(T[] a) {
if (a.length < size()) {
a = (T[]) new PseudoClass[size()];
}
int index = 0;
while(index < getBits().length) {
final long state = getBits()[index];
for(int bit=0; bit<Long.SIZE; bit++) {
long mask = 1l << bit;
if ((state & mask) == mask) {
int n = index * Long.SIZE + bit;
PseudoClass impl = getPseudoClass(n);
a[index++] = (T) impl;
}

}
}
return a;
}


@Override
public String toString() {
List<String> strings = new ArrayList<>();
Expand All @@ -94,12 +65,8 @@ public String toString() {
}

@Override
protected PseudoClass cast(Object o) {
if (o == null) {
throw new NullPointerException("null arg");
}
PseudoClass pseudoClass = (PseudoClass) o;
return pseudoClass;
protected Class<PseudoClass> getElementType() {
return PseudoClass.class;
}

@Override
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -68,8 +68,7 @@ public Key(Set<PseudoClass>[] pseudoClassStates, Font font) {

this.pseudoClassStates = new Set[pseudoClassStates.length];
for (int n=0; n<pseudoClassStates.length; n++) {
this.pseudoClassStates[n] = new PseudoClassState();
this.pseudoClassStates[n].addAll(pseudoClassStates[n]);
this.pseudoClassStates[n] = ImmutablePseudoClassSetsCache.of(pseudoClassStates[n]);
}
this.fontSize = font != null ? font.getSize() : Font.getDefault().getSize();

Expand All @@ -90,7 +89,7 @@ public int hashCode() {

hash = hashCode(fontSize);

final int iMax = pseudoClassStates != null ? pseudoClassStates.length : 0;
final int iMax = pseudoClassStates.length;

for (int i=0; i<iMax; i++) {

Expand Down Expand Up @@ -129,16 +128,6 @@ public boolean equals(Object obj) {
return false;
}

// either both must be null or both must be not-null
if ((pseudoClassStates == null) ^ (other.pseudoClassStates == null)) {
return false;
}

// if one is null, the other is too.
if (pseudoClassStates == null) {
return true;
}

if (pseudoClassStates.length != other.pseudoClassStates.length) {
return false;
}
Expand Down
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -58,35 +58,6 @@ public StyleClassSet() {

}

/** {@inheritDoc} */
@Override
public Object[] toArray() {
return toArray(new StyleClass[size()]);
}

/** {@inheritDoc} */
@Override
public <T> T[] toArray(T[] a) {
if (a.length < size()) {
a = (T[]) new StyleClass[size()];
}
int index = 0;
while(index < getBits().length) {
final long state = getBits()[index];
for(int bit=0; bit<Long.SIZE; bit++) {
long mask = 1l << bit;
if ((state & mask) == mask) {
int n = index * Long.SIZE + bit;
StyleClass impl = getStyleClass(n);
a[index++] = (T) impl;
}

}
}
return a;
}


@Override
public String toString() {
StringBuilder builder = new StringBuilder("style-classes: [");
Expand All @@ -102,12 +73,8 @@ public String toString() {
}

@Override
protected StyleClass cast(Object o) {
if (o == null) {
throw new NullPointerException("null arg");
}
StyleClass styleClass = (StyleClass) o;
return styleClass;
protected Class<StyleClass> getElementType() {
return StyleClass.class;
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions modules/javafx.graphics/src/main/java/javafx/css/Match.java
Expand Up @@ -27,7 +27,8 @@

import static javafx.geometry.NodeOrientation.INHERIT;

import java.util.Collections;
import com.sun.javafx.css.ImmutablePseudoClassSetsCache;

import java.util.Objects;
import java.util.Set;

Expand Down Expand Up @@ -58,7 +59,7 @@ public final class Match implements Comparable<Match> {
this.selector = selector;
this.idCount = idCount;
this.styleClassCount = styleClassCount;
this.pseudoClasses = Collections.unmodifiableSet(pseudoClasses);
this.pseudoClasses = ImmutablePseudoClassSetsCache.of(pseudoClasses);
int nPseudoClasses = pseudoClasses.size();
if (selector instanceof SimpleSelector simple) {
if (simple.getNodeOrientation() != INHERIT) {
Expand Down

1 comment on commit 5e145cc

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.