IDEA-CR-41536 1. clear string interner on close because thread reused later as part of application pool 2. doesn't make a lot of sense to intern class names

This commit is contained in:
Vladimir Krivosheev
2019-01-05 14:04:06 +01:00
parent 928d9cf7da
commit 40f2d3fd6a
8 changed files with 199 additions and 147 deletions
@@ -13,17 +13,13 @@ import com.intellij.openapi.extensions.ExtensionPoint;
import com.intellij.openapi.extensions.ExtensionsArea;
import com.intellij.openapi.extensions.PluginId;
import com.intellij.openapi.extensions.impl.ExtensionsAreaImpl;
import com.intellij.openapi.util.InvalidDataException;
import com.intellij.openapi.util.JDOMUtil;
import com.intellij.openapi.util.NullableLazyValue;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.*;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.openapi.util.text.StringUtilRt;
import com.intellij.util.ObjectUtils;
import com.intellij.util.SmartList;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.MultiMap;
import com.intellij.util.containers.StringInterner;
import com.intellij.util.xmlb.BeanBinding;
import com.intellij.util.xmlb.JDOMXIncluder;
import com.intellij.util.xmlb.XmlSerializer;
@@ -145,8 +141,8 @@ public class IdeaPluginDescriptorImpl implements IdeaPluginDescriptor {
readExternal(element);
}
public void loadFromFile(@NotNull File file, @Nullable StringInterner stringInterner) throws IOException, JDOMException {
readExternal(JDOMUtil.load(file, stringInterner), file.toURI().toURL(), JDOMXIncluder.DEFAULT_PATH_RESOLVER);
public void loadFromFile(@NotNull File file, @Nullable SafeJdomFactory factory) throws IOException, JDOMException {
readExternal(JDOMUtil.load(file, factory), file.toURI().toURL(), JDOMXIncluder.DEFAULT_PATH_RESOLVER);
}
// used in upsource
@@ -1,32 +1,53 @@
// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.ide.plugins;
import com.intellij.concurrency.JobSchedulerImpl;
import com.intellij.openapi.util.SafeJdomFactory;
import com.intellij.util.SmartList;
import com.intellij.util.concurrency.AppExecutorUtil;
import com.intellij.util.containers.StringInterner;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.Interner;
import org.jdom.*;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.ExecutorService;
final class LoadDescriptorsContext {
final class LoadDescriptorsContext implements AutoCloseable {
private final ExecutorService myExecutorService;
private final PluginLoadProgressManager myPluginLoadProgressManager;
private final Collection<Interner<String>> myInterners;
// synchronization will ruin parallel loading, so, string pool is local per thread
private final ThreadLocal<StringInterner> myThreadLocalStringInterner = ThreadLocal.withInitial(() -> new StringInterner() {
@NotNull
@Override
public String intern(@NotNull String name) {
// doesn't make any sense to intern long texts (JdomInternFactory doesn't intern CDATA, but plugin description can be simply Text)
return name.length() < 64 ? super.intern(name) : name;
}
});
private final ThreadLocal<SafeJdomFactory> myThreadLocalXmlFactory;
LoadDescriptorsContext(@Nullable PluginLoadProgressManager pluginLoadProgressManager, boolean isParallel) {
myPluginLoadProgressManager = pluginLoadProgressManager;
int maxThreads = isParallel ? JobSchedulerImpl.getCPUCoresCount() : 1;
myExecutorService = maxThreads > 1 ? AppExecutorUtil.createBoundedApplicationPoolExecutor("PluginManager Loader", maxThreads) : null;
int maxThreads = isParallel ? (Runtime.getRuntime().availableProcessors() - 1) : 1;
if (maxThreads > 1) {
myExecutorService = AppExecutorUtil.createBoundedApplicationPoolExecutor("PluginManager Loader", maxThreads);
myInterners = Collections.newSetFromMap(ContainerUtil.newConcurrentMap(maxThreads));
}
else {
myExecutorService = null;
myInterners = new SmartList<>();
}
myThreadLocalXmlFactory = ThreadLocal.withInitial(() -> {
Interner<String> interner = new Interner<String>(Arrays.asList(PluginXmlFactory.CLASS_NAMES)) {
@NotNull
@Override
public String intern(@NotNull String name) {
// doesn't make any sense to intern long texts (JdomInternFactory doesn't intern CDATA, but plugin description can be simply Text)
return name.length() < 64 ? super.intern(name) : name;
}
};
myInterners.add(interner);
return new PluginXmlFactory(interner);
});
}
@Nullable
@@ -40,7 +61,75 @@ final class LoadDescriptorsContext {
}
@Nullable
public StringInterner getStringInterner() {
return myThreadLocalStringInterner.get();
public SafeJdomFactory getXmlFactory() {
return myThreadLocalXmlFactory.get();
}
@Override
public void close() {
if (myExecutorService == null) {
myThreadLocalXmlFactory.remove();
return;
}
myExecutorService.submit(() -> {
for (Interner<String> interner : myInterners) {
interner.clear();
}
});
myExecutorService.shutdown();
}
/**
* Consider using some threshold in StringInterner - CDATA is not interned at all,
* but maybe some long text for Text node doesn't make sense to intern too.
*/
// don't intern CDATA - in most cases it is used for some unique large text (e.g. plugin description)
private final static class PluginXmlFactory extends SafeJdomFactory.BaseSafeJdomFactory {
// ouch, do we really cannot agree how to name implementation class attribute?
private static final String[] CLASS_NAMES = new String[]{
"implementation-class", "implementation", "implementationClass", "serviceImplementation", "class", "className", "beanClass",
"serviceInterface", "interface", "interfaceClass", "instance",
"qualifiedName",
};
private final Interner<String> stringInterner;
PluginXmlFactory(@NotNull Interner<String> stringInterner) {
this.stringInterner = stringInterner;
for (int i = 0; i < CLASS_NAMES.length; i++) {
CLASS_NAMES[i] = stringInterner.intern(CLASS_NAMES[i]);
}
}
@NotNull
@Override
public Element element(@NotNull String name, @Nullable Namespace namespace) {
return super.element(stringInterner.intern(name), namespace);
}
@NotNull
@Override
public Attribute attribute(@NotNull String name, @NotNull String value, @Nullable AttributeType type, @Nullable Namespace namespace) {
String internedName = stringInterner.intern(name);
for (String s : CLASS_NAMES) {
if (internedName == s) {
return super.attribute(internedName, value, type, namespace);
}
}
return super.attribute(internedName, stringInterner.intern(value), type, namespace);
}
@NotNull
@Override
public Text text(@NotNull String text, @NotNull Element parentElement) {
if (parentElement.getName() == "className" || parentElement.getName() == "implementation-class") {
return super.text(text, parentElement);
}
else {
return super.text(stringInterner.intern(text), parentElement);
}
}
}
}
@@ -26,7 +26,6 @@ import com.intellij.util.*;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.JBIterable;
import com.intellij.util.containers.MultiMap;
import com.intellij.util.containers.StringInterner;
import com.intellij.util.execution.ParametersListUtil;
import com.intellij.util.graph.*;
import com.intellij.util.io.URLUtil;
@@ -646,7 +645,7 @@ public class PluginManagerCore {
try {
IdeaPluginDescriptorImpl descriptor = new IdeaPluginDescriptorImpl(notNull(pluginPath, file), loadingContext.isBundled);
descriptor.loadFromFile(descriptorFile, loadingContext.getStringInterner());
descriptor.loadFromFile(descriptorFile, loadingContext.getXmlFactory());
return descriptor;
}
catch (XmlSerializationException | JDOMException | IOException e) {
@@ -681,7 +680,7 @@ public class PluginManagerCore {
ZipEntry entry = zipFile.getEntry(entryName);
if (entry != null) {
IdeaPluginDescriptorImpl descriptor = new IdeaPluginDescriptorImpl(notNull(pluginPath, file), context.isBundled);
descriptor.readExternal(JDOMUtil.load(zipFile.getInputStream(entry), context.getStringInterner()), jarURL, pathResolver);
descriptor.readExternal(JDOMUtil.load(zipFile.getInputStream(entry), context.getXmlFactory()), jarURL, pathResolver);
context.myLastZipFileContainingDescriptor = file;
return descriptor;
}
@@ -745,11 +744,11 @@ public class PluginManagerCore {
}
@Nullable
public StringInterner getStringInterner() {
public SafeJdomFactory getXmlFactory() {
if (myParentContext == null) {
return null;
}
return myParentContext.getStringInterner();
return myParentContext.getXmlFactory();
}
}
@@ -1205,8 +1204,7 @@ public class PluginManagerCore {
URL platformPluginURL = computePlatformPluginUrlAndCollectPluginUrls(PluginManagerCore.class.getClassLoader(), urlsFromClassPath);
PluginLoadProgressManager pluginLoadProgressManager = progress == null ? null : new PluginLoadProgressManager(progress, urlsFromClassPath.size());
LoadDescriptorsContext context = new LoadDescriptorsContext(pluginLoadProgressManager, SystemProperties.getBooleanProperty("parallel.pluginDescriptors.loading", true));
try {
try (LoadDescriptorsContext context = new LoadDescriptorsContext(pluginLoadProgressManager, SystemProperties.getBooleanProperty("parallel.pluginDescriptors.loading", true))) {
loadDescriptorsFromDir(new File(PathManager.getPluginsPath()), result, false, context);
Application application = ApplicationManager.getApplication();
if (application == null || !application.isUnitTestMode()) {
@@ -6,7 +6,6 @@ import com.intellij.openapi.util.io.FileUtil;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.openapi.vfs.CharsetToolkit;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.StringInterner;
import com.intellij.util.io.URLUtil;
import com.intellij.util.text.CharArrayUtil;
import com.intellij.util.text.CharSequenceReader;
@@ -15,6 +14,7 @@ import org.jdom.*;
import org.jdom.filter.Filter;
import org.jdom.output.Format;
import org.jdom.output.XMLOutputter;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -215,11 +215,11 @@ public class JDOMUtil {
}
@NotNull
private static Element loadUsingStaX(@NotNull Reader reader, @Nullable StringInterner stringInterner) throws JDOMException, IOException {
private static Element loadUsingStaX(@NotNull Reader reader, @Nullable SafeJdomFactory factory) throws JDOMException, IOException {
try {
XMLStreamReader xmlStreamReader = XML_INPUT_FACTORY.getValue().createXMLStreamReader(reader);
try {
return SafeStAXStreamBuilder.build(xmlStreamReader, true, stringInterner);
return SafeStAXStreamBuilder.build(xmlStreamReader, true, factory == null ? SafeStAXStreamBuilder.FACTORY : factory);
}
finally {
xmlStreamReader.close();
@@ -271,12 +271,12 @@ public class JDOMUtil {
}
/**
* Consider using some threshold in StringInterner - CDATA is not interned at all,
* but maybe some long text for Text node doesn't make sense to intern too.
* Internal use only.
*/
@ApiStatus.Experimental
@NotNull
public static Element load(@NotNull File file, @Nullable StringInterner stringInterner) throws JDOMException, IOException {
return loadUsingStaX(new BufferedReader(new InputStreamReader(new FileInputStream(file), CharsetToolkit.UTF8_CHARSET)), stringInterner);
public static Element load(@NotNull File file, @Nullable SafeJdomFactory factory) throws JDOMException, IOException {
return loadUsingStaX(new BufferedReader(new InputStreamReader(new FileInputStream(file), CharsetToolkit.UTF8_CHARSET)), factory);
}
/**
@@ -301,12 +301,12 @@ public class JDOMUtil {
}
/**
* Consider using some threshold in StringInterner - CDATA is not interned at all,
* but maybe some long text for Text node doesn't make sense to intern too.
* Internal use only.
*/
@ApiStatus.Experimental
@NotNull
public static Element load(@NotNull InputStream stream, @Nullable StringInterner stringInterner) throws JDOMException, IOException {
return loadUsingStaX(new InputStreamReader(stream, CharsetToolkit.UTF8_CHARSET), stringInterner);
public static Element load(@NotNull InputStream stream, @Nullable SafeJdomFactory factory) throws JDOMException, IOException {
return loadUsingStaX(new InputStreamReader(stream, CharsetToolkit.UTF8_CHARSET), factory);
}
@NotNull
@@ -1,60 +0,0 @@
// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.openapi.util;
import com.intellij.util.containers.StringInterner;
import org.jdom.*;
import org.jetbrains.annotations.NotNull;
// don't intern CDATA - in most cases it is used for some unique large text (e.g. plugin description)
final class JdomInternFactory extends DefaultJDOMFactory {
private final StringInterner stringInterner;
JdomInternFactory(@NotNull StringInterner stringInterner) {
this.stringInterner = stringInterner;
}
@Override
public Attribute attribute(String name, String value, Namespace namespace) {
return super.attribute(stringInterner.intern(name), stringInterner.intern(value), namespace);
}
@Override
public Attribute attribute(String name, String value) {
return super.attribute(stringInterner.intern(name), stringInterner.intern(value));
}
@Override
public Attribute attribute(String name, String value, AttributeType type) {
return super.attribute(stringInterner.intern(name), stringInterner.intern(value), type);
}
@Override
public Attribute attribute(String name, String value, AttributeType type, Namespace namespace) {
return super.attribute(stringInterner.intern(name), stringInterner.intern(value), type, namespace);
}
@Override
public Text text(int line, int col, String text) {
return super.text(line, col, stringInterner.intern(text));
}
@Override
public Element element(int line, int col, String name, Namespace namespace) {
return super.element(line, col, stringInterner.intern(name), namespace);
}
@Override
public Element element(int line, int col, String name) {
return super.element(line, col, stringInterner.intern(name));
}
@Override
public Element element(int line, int col, String name, String uri) {
return super.element(line, col, stringInterner.intern(name), uri);
}
@Override
public Element element(int line, int col, String name, String prefix, String uri) {
return super.element(line, col, stringInterner.intern(name), prefix, uri);
}
}
@@ -0,0 +1,55 @@
// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.openapi.util;
import org.jdom.*;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
* Internal use only.
*/
@ApiStatus.Experimental
public interface SafeJdomFactory {
@NotNull
Element element(@NotNull String name, @Nullable Namespace namespace);
@NotNull
Attribute attribute(@NotNull String name, @NotNull String value, @Nullable AttributeType type, @Nullable Namespace namespace);
@NotNull
Text text(@NotNull String text, @NotNull Element parentElement);
@NotNull
CDATA cdata(@NotNull String text);
class BaseSafeJdomFactory implements SafeJdomFactory {
@NotNull
@Override
public Element element(@NotNull String name, @Nullable Namespace namespace) {
Element element = new Element(name, namespace);
if (namespace != null) {
element.setNamespace(namespace);
}
return element;
}
@NotNull
@Override
public Attribute attribute(@NotNull String name, @NotNull String value, @Nullable AttributeType type, @Nullable Namespace namespace) {
return new Attribute(name, value, type, namespace);
}
@NotNull
@Override
public Text text(@NotNull String text, @NotNull Element parentElement) {
return new Text(text);
}
@NotNull
@Override
public CDATA cdata(@NotNull String text) {
return new CDATA(text);
}
}
}
@@ -1,10 +1,8 @@
// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.openapi.util;
import com.intellij.util.containers.StringInterner;
import org.jdom.*;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
@@ -13,16 +11,16 @@ import static javax.xml.stream.XMLStreamConstants.*;
// DTD, COMMENT and PROCESSING_INSTRUCTION wi
final class SafeStAXStreamBuilder {
private static final JDOMFactory factory = new DefaultJDOMFactory();
static final SafeJdomFactory FACTORY = new SafeJdomFactory.BaseSafeJdomFactory();
static Document buildDocument(@NotNull XMLStreamReader stream, boolean isIgnoreBoundaryWhitespace) throws JDOMException, XMLStreamException {
static Document buildDocument(@NotNull XMLStreamReader stream, @SuppressWarnings("SameParameterValue") boolean isIgnoreBoundaryWhitespace) throws JDOMException, XMLStreamException {
int state = stream.getEventType();
if (START_DOCUMENT != state) {
throw new JDOMException("JDOM requires that XMLStreamReaders are at their beginning when being processed.");
}
final Document document = factory.document(null);
final Document document = new Document();
while (state != END_DOCUMENT) {
switch (state) {
@@ -35,22 +33,15 @@ final class SafeStAXStreamBuilder {
break;
case DTD:
case COMMENT:
case PROCESSING_INSTRUCTION:
case SPACE:
break;
case START_ELEMENT:
document.setRootElement(processElementFragment(stream, isIgnoreBoundaryWhitespace, factory));
document.setRootElement(processElementFragment(stream, isIgnoreBoundaryWhitespace, FACTORY));
break;
case END_ELEMENT:
throw new JDOMException("Unexpected XMLStream event at Document level: END_ELEMENT");
case ENTITY_REFERENCE:
throw new JDOMException("Unexpected XMLStream event at Document level: ENTITY_REFERENCE");
case CDATA:
throw new JDOMException("Unexpected XMLStream event at Document level: CDATA");
case SPACE:
// Can happen when XMLInputFactory2.P_REPORT_PROLOG_WHITESPACE is set to true
document.addContent(factory.text(stream.getText()));
break;
case CHARACTERS:
final String badTxt = stream.getText();
if (!Verifier.isAllXMLWhitespace(badTxt)) {
@@ -59,12 +50,8 @@ final class SafeStAXStreamBuilder {
// otherwise ignore the chars.
break;
case COMMENT:
case PROCESSING_INSTRUCTION:
break;
default:
throw new JDOMException("Unexpected XMLStream event " + state);
throw new JDOMException("Unexpected XMLStream event at Document level:" + state);
}
if (stream.hasNext()) {
@@ -77,15 +64,13 @@ final class SafeStAXStreamBuilder {
return document;
}
static Element build(@NotNull XMLStreamReader stream, boolean isIgnoreBoundaryWhitespace, @Nullable StringInterner stringInterner) throws JDOMException, XMLStreamException {
static Element build(@NotNull XMLStreamReader stream, @SuppressWarnings("SameParameterValue") boolean isIgnoreBoundaryWhitespace, @NotNull SafeJdomFactory factory) throws JDOMException, XMLStreamException {
int state = stream.getEventType();
if (START_DOCUMENT != state) {
throw new JDOMException("JDOM requires that XMLStreamReaders are at their beginning when being processed");
}
JDOMFactory factory = stringInterner == null ? SafeStAXStreamBuilder.factory : new JdomInternFactory(stringInterner);
Element rootElement = null;
while (state != END_DOCUMENT) {
switch (state) {
@@ -120,7 +105,7 @@ final class SafeStAXStreamBuilder {
return rootElement;
}
private static Element processElementFragment(@NotNull XMLStreamReader reader, boolean isIgnoreBoundaryWhitespace, @NotNull JDOMFactory factory) throws XMLStreamException, JDOMException {
private static Element processElementFragment(@NotNull XMLStreamReader reader, boolean isIgnoreBoundaryWhitespace, @NotNull SafeJdomFactory factory) throws XMLStreamException, JDOMException {
if (reader.getEventType() != START_ELEMENT) {
throw new JDOMException("JDOM requires that the XMLStreamReader is at the START_ELEMENT state when retrieving an Element Fragment.");
}
@@ -146,19 +131,16 @@ final class SafeStAXStreamBuilder {
case SPACE:
if (!isIgnoreBoundaryWhitespace) {
current.addContent(factory.text(reader.getText()));
current.addContent(factory.text(reader.getText(), current));
}
case CHARACTERS:
if (!isIgnoreBoundaryWhitespace || !reader.isWhiteSpace()) {
current.addContent(factory.text(reader.getText()));
current.addContent(factory.text(reader.getText(), current));
}
break;
case ENTITY_REFERENCE:
current.addContent(factory.entityRef(reader.getLocalName()));
break;
case COMMENT:
case PROCESSING_INSTRUCTION:
break;
@@ -172,12 +154,12 @@ final class SafeStAXStreamBuilder {
}
@NotNull
private static Element processElement(@NotNull XMLStreamReader reader, @NotNull JDOMFactory factory) {
private static Element processElement(@NotNull XMLStreamReader reader, @NotNull SafeJdomFactory factory) {
final Element element = factory.element(reader.getLocalName(), Namespace.getNamespace(reader.getPrefix(), reader.getNamespaceURI()));
// Handle attributes
for (int i = 0, len = reader.getAttributeCount(); i < len; i++) {
factory.setAttribute(element, factory.attribute(
element.setAttribute(factory.attribute(
reader.getAttributeLocalName(i),
reader.getAttributeValue(i),
AttributeType.getAttributeType(reader.getAttributeType(i)),
@@ -1,23 +1,10 @@
/*
* Copyright 2000-2015 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.util.containers;
import gnu.trove.TObjectHashingStrategy;
import org.jetbrains.annotations.NotNull;
import java.util.Collection;
import java.util.Set;
/**
@@ -34,6 +21,11 @@ public class Interner<T> {
public Interner() {
mySet = new OpenTHashSet<T>();
}
public Interner(@NotNull Collection<? extends T> initialItems) {
mySet = new OpenTHashSet<T>(initialItems);
}
public Interner(@NotNull TObjectHashingStrategy<T> strategy) {
mySet = new OpenTHashSet<T>(strategy);
}