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

WildFly - ConcurrentHashMap over synchronized collections and some improvements #6602

Merged
merged 1 commit into from
Oct 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ public static WildflyClassLoader createWildFlyClassLoader(String serverRoot) {
private static void findJarAddUrl(List<URL> urlList, Path folder, String pathMatcherRaw) {
PathMatcher pathMatcher = FileSystems.getDefault().getPathMatcher(pathMatcherRaw);

Stream<Path> walk = null;
try {
walk = Files.walk(folder);
try (Stream<Path> walk = Files.walk(folder)) {
Optional<Path> firstJarMatching = walk
.filter(pathMatcher::matches)
.findFirst();
Expand All @@ -115,10 +113,6 @@ private static void findJarAddUrl(List<URL> urlList, Path folder, String pathMat
}
} catch (IOException ex) {
LOGGER.log(Level.INFO, null, ex);
} finally {
if (walk != null) {
walk.close();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.enterprise.deploy.model.DeployableObject;
Expand Down Expand Up @@ -83,8 +83,8 @@ public class WildflyDeploymentManager implements DeploymentManager2 {
* server instance bcs instance properties are also removed along with
* instance.
*/
private static final Map<InstanceProperties, Boolean> PROPERTIES_TO_IS_RUNNING
= Collections.synchronizedMap(new WeakHashMap());
private static final ConcurrentMap<InstanceProperties, Boolean> PROPERTIES_TO_IS_RUNNING
= new ConcurrentHashMap(new WeakHashMap());

Choose a reason for hiding this comment

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

Hmm, isn't this very different than the original?

new ConcurrentHashMap(new WeakHashMap()) -> just creates a new ConcurrentHashMap, giving new WeakHashMap (empty map) as a parameter does nothing.

Collections.synchronizedMap(new WeakHashMap()) actually creates a synchronized version of WeakHashMap.

If ConcurrentHashMap is used, one must change the usage so that the keys inserted in the map are weak references. IF the original behavior is wanted. Right?

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right - would you be willing to create a PR for your suggested fix?

Choose a reason for hiding this comment

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

You are right - would you be willing to create a PR for your suggested fix?

I think it is faster if you guys do it. You can choose the preferred implementation also :)

Copy link
Contributor

Choose a reason for hiding this comment

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

See #6872


private final DeploymentFactory df;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,8 @@ public Set<MessageDestination> getMessageDestinations() throws ConfigurationExce
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParser parser = factory.newSAXParser();
WildflyMessageDestinationHandler handler = new WildflyMessageDestinationHandler();
InputStream is = new BufferedInputStream(configFile.getInputStream());
try {
try (InputStream is = new BufferedInputStream(configFile.getInputStream())) {
parser.parse(is, handler);
} finally {
is.close();
}
messageDestinations.addAll(handler.getMessageDestinations());
} catch (IOException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.enterprise.deploy.spi.DeploymentManager;
import javax.swing.JFileChooser;
import javax.swing.JPanel;
Expand Down Expand Up @@ -85,16 +86,12 @@ String getParentDirectory() {

// Event handling
//
private final Set<ChangeListener> listeners = new HashSet<>(1);
private final Set<ChangeListener> listeners = ConcurrentHashMap.newKeySet(2);
public final void addChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.add(l);
}
listeners.add(l);
}
public final void removeChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.remove(l);
}
listeners.remove(l);
}
protected final void fireChangeEvent() {
Iterator<ChangeListener> it;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,23 @@ private String resolveImplementationVersion() throws IOException {
File wsToolsJar = new File(root, JAXWS_TOOLS_JAR);

if (wsToolsJar.exists()) {
JarFile jarFile = new JarFile(wsToolsJar);
JarEntry entry = jarFile.getJarEntry("com/sun/tools/ws/version.properties"); //NOI18N
if (entry != null) {
InputStream is = jarFile.getInputStream(entry);
BufferedReader r = new BufferedReader(new InputStreamReader(is));
String ln = null;
String ver = null;
while ((ln=r.readLine()) != null) {
String line = ln.trim();
if (line.startsWith("major-version=")) { //NOI18N
ver = line.substring(14);
try (JarFile jarFile = new JarFile(wsToolsJar)) {
JarEntry entry = jarFile.getJarEntry("com/sun/tools/ws/version.properties"); //NOI18N
if (entry != null) {
String ver = null;
try (InputStream is = jarFile.getInputStream(entry);
BufferedReader r = new BufferedReader(new InputStreamReader(is))) {
String ln = null;
while ((ln=r.readLine()) != null) {
String line = ln.trim();
if (line.startsWith("major-version=")) { //NOI18N
ver = line.substring(14);
break;
}
}
}
return ver;
}
r.close();
return ver;
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public synchronized J2eePlatformImpl getJ2eePlatformImpl(DeploymentManager dm) {

public static class J2eePlatformImplImpl extends J2eePlatformImpl2 {

private static final Set<Type> MODULE_TYPES = new HashSet<Type>();
private static final Set<Type> MODULE_TYPES = new HashSet<Type>(8);

static {
MODULE_TYPES.add(Type.EAR);
Expand All @@ -101,7 +101,7 @@ public static class J2eePlatformImplImpl extends J2eePlatformImpl2 {
MODULE_TYPES.add(Type.CAR);
}

private static final Set<Profile> WILDFLY_PROFILES = new HashSet<Profile>();
private static final Set<Profile> WILDFLY_PROFILES = new HashSet<Profile>(16);

static {
WILDFLY_PROFILES.add(Profile.JAVA_EE_6_WEB);
Expand All @@ -112,21 +112,21 @@ public static class J2eePlatformImplImpl extends J2eePlatformImpl2 {
WILDFLY_PROFILES.add(Profile.JAVA_EE_8_FULL);
WILDFLY_PROFILES.add(Profile.JAKARTA_EE_8_FULL);
}
private static final Set<Profile> JAKARTAEE_FULL_PROFILES = new HashSet<Profile>();
private static final Set<Profile> JAKARTAEE_FULL_PROFILES = new HashSet<Profile>(8);

static {
JAKARTAEE_FULL_PROFILES.add(Profile.JAKARTA_EE_9_FULL);
JAKARTAEE_FULL_PROFILES.add(Profile.JAKARTA_EE_9_1_FULL);
JAKARTAEE_FULL_PROFILES.add(Profile.JAKARTA_EE_10_FULL);
}
private static final Set<Profile> EAP6_PROFILES = new HashSet<Profile>();
private static final Set<Profile> EAP6_PROFILES = new HashSet<Profile>(4);

static {
EAP6_PROFILES.add(Profile.JAVA_EE_6_WEB);
EAP6_PROFILES.add(Profile.JAVA_EE_6_FULL);
}

private static final Set<Profile> WILDFLY_WEB_PROFILES = new HashSet<Profile>();
private static final Set<Profile> WILDFLY_WEB_PROFILES = new HashSet<Profile>(16);

static {
WILDFLY_WEB_PROFILES.add(Profile.JAVA_EE_6_WEB);
Expand All @@ -138,7 +138,7 @@ public static class J2eePlatformImplImpl extends J2eePlatformImpl2 {
WILDFLY_WEB_PROFILES.add(Profile.JAKARTA_EE_10_WEB);
}

private static final Set<Profile> JAKARTAEE_WEB_PROFILES = new HashSet<Profile>();
private static final Set<Profile> JAKARTAEE_WEB_PROFILES = new HashSet<Profile>(8);

static {
JAKARTAEE_WEB_PROFILES.add(Profile.JAKARTA_EE_9_WEB);
Expand Down Expand Up @@ -188,7 +188,7 @@ public Set<Type> getSupportedTypes() {
}

@Override
public Set/*<String>*/ getSupportedJavaPlatformVersions() {
public Set<String> getSupportedJavaPlatformVersions() {
Set versions = new HashSet();
versions.add("1.7"); // NOI18N
versions.add("1.8"); // NOI18N
Expand All @@ -198,6 +198,9 @@ public Set<Type> getSupportedTypes() {
if (this.properties.getServerVersion().compareToIgnoreUpdate(WildflyPluginUtils.EAP_7_0) >= 0) {
versions.add("17"); // NOI18N
}
if (this.properties.getServerVersion().compareToIgnoreUpdate(WildflyPluginUtils.WILDFLY_30_0_0) >= 0) {
versions.add("21"); // NOI18N
}
return versions;
}

Expand Down Expand Up @@ -383,21 +386,16 @@ private static boolean containsService(LibraryImplementation library, String ser
}

private static boolean containsService(FileObject serviceFO, String serviceName, String serviceImplName) {
try {
BufferedReader br = new BufferedReader(new InputStreamReader(serviceFO.getInputStream()));
try {
String line;
while ((line = br.readLine()) != null) {
int ci = line.indexOf('#');
if (ci >= 0) {
line = line.substring(0, ci);
}
if (line.trim().equals(serviceImplName)) {
return true;
}
try (BufferedReader br = new BufferedReader(new InputStreamReader(serviceFO.getInputStream()))) {
String line;
while ((line = br.readLine()) != null) {
int ci = line.indexOf('#');
if (ci >= 0) {
line = line.substring(0, ci);
}
if (line.trim().equals(serviceImplName)) {
return true;
}
} finally {
br.close();
}
} catch (Exception ex) {
Exceptions.attachLocalizedMessage(ex, serviceFO.toURL().toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.netbeans.modules.j2ee.deployment.plugins.spi.StartServer;
import org.openide.util.RequestProcessor;
import java.util.Vector;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.enterprise.deploy.spi.DeploymentManager;
Expand Down Expand Up @@ -78,8 +79,8 @@ static enum ACTION_STATUS {

private boolean consoleConfigured = false;

private static final Set<String> IS_DEBUG_MODE_URI = Collections.synchronizedSet(
new HashSet<String>(AVERAGE_SERVER_INSTANCES));
private static final Set<String> IS_DEBUG_MODE_URI =
ConcurrentHashMap.newKeySet(AVERAGE_SERVER_INSTANCES);

public WildflyStartServer(DeploymentManager dm) {
this.dm = (WildflyDeploymentManager) dm;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;
import org.netbeans.api.java.platform.JavaPlatformManager;
Expand All @@ -43,7 +44,7 @@ public class AddServerLocationPanel implements WizardDescriptor.FinishablePanel,

private AddServerLocationVisualPanel component;
private WizardDescriptor wizard;
private final transient Set listeners = new HashSet(1);
private final transient Set listeners = ConcurrentHashMap.newKeySet(2);

public AddServerLocationPanel(WildflyInstantiatingIterator instantiatingIterator) {
this.instantiatingIterator = instantiatingIterator;
Expand Down Expand Up @@ -113,16 +114,12 @@ public boolean isValid() {

@Override
public void removeChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.remove(l);
}
listeners.remove(l);
}

@Override
public void addChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.add(l);
}
listeners.add(l);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.swing.JFileChooser;
import javax.swing.SwingUtilities;
import javax.swing.event.ChangeEvent;
Expand All @@ -41,7 +42,7 @@
*/
public class AddServerLocationVisualPanel extends javax.swing.JPanel {

private final Set listeners = new HashSet();
private final Set listeners = ConcurrentHashMap.newKeySet();

/**
* Creates new form AddServerLocationVisualPanel
Expand Down Expand Up @@ -92,15 +93,11 @@ public String getConfigurationLocation() {
}

public void addChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.add(l);
}
listeners.add(l);
}

public void removeChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.remove(l);
}
listeners.remove(l);
}

private void fireChangeEvent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;
import org.netbeans.modules.j2ee.deployment.plugins.api.InstanceProperties;
Expand Down Expand Up @@ -181,19 +182,15 @@ private void fireChangeEvent(ChangeEvent ev) {
}
}

private final transient Set listeners = new HashSet(1);
private final transient Set listeners = ConcurrentHashMap.newKeySet(2);
@Override
public void removeChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.remove(l);
}
listeners.remove(l);
}

@Override
public void addChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.add(l);
}
listeners.add(l);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.swing.AbstractListModel;
import javax.swing.ComboBoxModel;
import javax.swing.JComboBox;
Expand All @@ -47,7 +48,7 @@
*/
public class AddServerPropertiesVisualPanel extends JPanel {

private final Set listeners = new HashSet();
private final Set listeners = ConcurrentHashMap.newKeySet();

private javax.swing.JComboBox domainField; // Domain name (list of registered domains) can be edited
private javax.swing.JTextField domainPathField; //
Expand Down Expand Up @@ -77,15 +78,11 @@ public AddServerPropertiesVisualPanel() {
}

public void addChangeListener(ChangeListener l) {
synchronized (listeners) {
listeners.add(l);
}
listeners.add(l);
}

public void removeChangeListener(ChangeListener l ) {
synchronized (listeners) {
listeners.remove(l);
}
listeners.remove(l);
}

private void somethingChanged() {
Expand Down
Loading