Skip to content

Commit

Permalink
[performance] O(n) StringLiteral concatenation
Browse files Browse the repository at this point in the history 
ExtendedStringLiteral.extendWith(StringLiteral):65 had O(n^2) memory
consumption.
Use a linked list implementation instead and defer char[] concatenation
until needed.

https://bugs.eclipse.org/bugs/show_bug.cgi?id=574433
  • Loading branch information
@EcljpseB0T @jukzi
EcljpseB0T authored and jukzi committed Jan 15, 2024
1 parent 45b012d commit ca12584
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 45 deletions.
6 changes: 3 additions & 3 deletions ....core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ExtendedStringLiteral.java
View file
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2009 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -18,8 +18,8 @@

public class ExtendedStringLiteral extends StringLiteral {

public ExtendedStringLiteral(char[] token, int start, int end, int lineNumber) {
super(token, start, end, lineNumber);
protected ExtendedStringLiteral(StringLiteral optionalHead, Object sourcesTail, int start, int end, int lineNumber) {
super(optionalHead, sourcesTail, start, end, lineNumber);
}

@Override
Expand Down
79 changes: 57 additions & 22 deletions ...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/StringLiteral.java
View file
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2014 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -13,8 +13,6 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.ast;

import java.util.Arrays;

import org.eclipse.jdt.internal.compiler.ASTVisitor;
import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
import org.eclipse.jdt.internal.compiler.impl.StringConstant;
Expand All @@ -24,42 +22,50 @@

public class StringLiteral extends Literal {

private char[] source;
/** may be null, prefixes tail **/
protected StringLiteral optionalHead;
/** StringLiteral or char[] tail **/
protected Object tail;
/** zero based **/
private final int lineNumber;

public StringLiteral(char[] token, int start, int end, int lineNumber) {
public StringLiteral(char[] token, int start, int end, int lineNumber1based) {
this(null, token, start, end, lineNumber1based - 1);
}

protected StringLiteral(StringLiteral optionalHead, Object tail, int start, int end, int lineNumber) {
super(start, end);
this.source = token;
this.lineNumber = lineNumber - 1; // line number is 1 based
this.optionalHead = optionalHead;
this.tail = tail;
this.lineNumber = lineNumber;
}

public StringLiteral(int s, int e) {
this(null, s, e, 1);
this(null, null, s, e, 0);
}

@Override
public void computeConstant() {
this.constant = StringConstant.fromValue(String.valueOf(this.source));
this.constant = StringConstant.fromValue(String.valueOf(this.source()));
}

/**
* creates a copy of dedicated Type for optimizeStringLiterals with the given CharLiteral appended
*/
public ExtendedStringLiteral extendWith(CharLiteral lit) {
return new ExtendedStringLiteral(append(this.source(), new char[] { lit.value }),
this.sourceStart, lit.sourceEnd, this.getLineNumber() + 1);
char[] charTail = new char[] { lit.value };
return new ExtendedStringLiteral(this, charTail, this.sourceStart, lit.sourceEnd, this.getLineNumber());
}

/**
* creates a copy of dedicated Type for optimizeStringLiterals with the given StringLiteral appended
*/
public ExtendedStringLiteral extendWith(StringLiteral lit) {
return new ExtendedStringLiteral(append(this.source(), lit.source()), this.sourceStart,
lit.sourceEnd, this.getLineNumber() + 1);
}
protected static char[] append(char[] source, char[] source2) {
char[] result = Arrays.copyOfRange(source, 0, source.length + source2.length);
System.arraycopy(source2, 0, result, source.length, source2.length);
return result;
return new ExtendedStringLiteral(this, lit, this.sourceStart, lit.sourceEnd, this.getLineNumber());
}


/**
* Add the lit source to mine, just as if it was mine
* creates a copy of dedicated Type for unoptimizeStringLiterals with the given StringLiteral appended
*/
public StringLiteralConcatenation extendsWith(StringLiteral lit) {
return new StringLiteralConcatenation(this, lit);
Expand Down Expand Up @@ -94,7 +100,35 @@ public StringBuilder printExpression(int indent, StringBuilder output) {

@Override
public char[] source() {
return Arrays.copyOf(this.source, this.source.length);
if (this.optionalHead == null && this.tail instanceof char[] ch) {
// fast path without copy
return ch;
}
// flatten linked list to char[]
int size = append(null, 0, this);
char[] result = new char[size];
append(result, 0, this);
flatten(result);
return result;
}

protected void flatten(char[] result) {
setSource(result); // keep flat version
}

private static int append(char[] result, int length, StringLiteral o) {
do {
if (o.tail instanceof char[] c) {
if (result != null) {
System.arraycopy(c, 0, result, result.length - c.length - length, c.length);
}
length += c.length;
} else {
length = append(result, length, ((StringLiteral) o.tail));
}
o = o.optionalHead;
} while (o != null);
return length;
}

@Override
Expand All @@ -104,7 +138,8 @@ public void traverse(ASTVisitor visitor, BlockScope scope) {
}

public void setSource(char[] source) {
this.source = source;
this.tail = source;
this.optionalHead = null;
}

public int getLineNumber() {
Expand Down
57 changes: 37 additions & 20 deletions ....compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/StringLiteralConcatenation.java
View file
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2009 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -13,34 +13,19 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.ast;

import java.util.Arrays;

import org.eclipse.jdt.internal.compiler.ASTVisitor;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;

/**
* Flatten string literal
*/
public class StringLiteralConcatenation extends StringLiteral {
private static final int INITIAL_SIZE = 5;
private final StringLiteral[] literals;
private final int counter;

/**
* Build a two-strings literal
*/
public StringLiteralConcatenation(StringLiteral str1, StringLiteral str2) {
super(StringLiteral.append(str1.source(), str2.source()), str1.sourceStart, str2.sourceEnd,
str1.getLineNumber() + 1);
if (str1 instanceof StringLiteralConcatenation s1) {
this.literals = Arrays.copyOf(s1.literals, s1.literals.length + 1);
this.counter = s1.counter + 1;
} else {
this.literals = new StringLiteral[INITIAL_SIZE];
this.literals[0] = str1;
this.counter = 2;
}
this.literals[this.counter - 1] = str2;
public StringLiteralConcatenation(StringLiteral str1, StringLiteral lit) {
super(str1, lit, str1.sourceStart, lit.sourceEnd, str1.getLineNumber());
}

@Override
Expand All @@ -64,6 +49,38 @@ public void traverse(ASTVisitor visitor, BlockScope scope) {
}

public StringLiteral[] getLiterals() {
return Arrays.copyOf(this.literals, this.counter);
int size = append(null, 0, this);
StringLiteral[] result = new StringLiteral[size];
append(result, 0, this);
return result;
}

private static int append(StringLiteral[] result, int length, StringLiteral o) {
do {
if (o.tail instanceof StringLiteral l) {
if (result != null) {
result[result.length - 1 - length] = l;
}
length += 1;
} else {
if (result != null) {
result[result.length - 1 - length] = o;
}
length += 1;
}
o = o.optionalHead;
} while (o != null);
return length;
}

@Override
protected void flatten(char[] result) {
// don't store flattened representation: getLiterals() still needs the linked list
}

@Override
public void setSource(char[] source) {
throw new UnsupportedOperationException();
}
}

}

0 comments on commit ca12584

Please sign in to comment.