Skip to content

Commit 4e6c224

Browse files
jisedlacjisedlac
committed
Improved JMX heartbeat responsiveness, cleanup.
1 parent 7fb41dc commit 4e6c224

File tree

2 files changed

+228
-116
lines changed

2 files changed

+228
-116
lines changed

‎visualvm/jmx/src/org/graalvm/visualvm/jmx/impl/JmxApplicationProvider.java

Lines changed: 41 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.graalvm.visualvm.core.options.GlobalPreferences;
4343
import org.graalvm.visualvm.jmx.JmxApplicationException;
4444
import org.graalvm.visualvm.jmx.JmxApplicationsSupport;
45-
import org.graalvm.visualvm.tools.jmx.JmxModel;
4645
import org.graalvm.visualvm.tools.jmx.JmxModel.ConnectionState;
4746
import org.graalvm.visualvm.tools.jmx.JmxModelFactory;
4847
import java.awt.BorderLayout;
@@ -54,11 +53,12 @@
5453
import java.net.InetAddress;
5554
import java.net.MalformedURLException;
5655
import java.net.NetworkInterface;
56+
import java.util.ArrayList;
57+
import java.util.Collection;
5758
import java.util.Collections;
5859
import java.util.Enumeration;
5960
import java.util.HashMap;
6061
import java.util.HashSet;
61-
import java.util.Iterator;
6262
import java.util.Map;
6363
import java.util.Set;
6464
import java.util.concurrent.atomic.AtomicInteger;
@@ -350,127 +350,49 @@ private JmxApplication addLazyJmxApplication(boolean newApp, JMXServiceURL servi
350350
// // If everything succeeded, add datasource to application tree
351351
host.getRepository().addDataSource(application);
352352

353-
// if (model == null || model.getConnectionState() != ConnectionState.CONNECTED) {
354-
synchronized (unavailableApps) { unavailableApps.add(application); }
355-
if (scheduleHeartbeat) scheduleHeartbeat();
356-
// }
353+
if (scheduleHeartbeat) JmxHeartbeat.scheduleImmediately(application);
357354

358355
return application;
359356
}
360357

361358

362-
private static final int HEARTBEAT_DELAY = 100;
363-
private static final int HEARTBEAT_POLL_DELAY = 5000;
364-
private static final int HEARTBEAT_MAX_THREADS = 10;
365-
private final Set<JmxApplication> unavailableApps = new HashSet();
366-
367-
private volatile boolean heartbeatRunning;
368-
private volatile boolean anotherHeartbeatPending;
369-
370-
private void scheduleHeartbeat() {
371-
scheduleHeartbeatImpl(true);
372-
}
373-
374-
private void scheduleHeartbeatImpl(boolean immediately) {
375-
if (heartbeatRunning) {
376-
anotherHeartbeatPending = true;
377-
return;
378-
} else {
379-
heartbeatRunning = true;
380-
}
381-
382-
Set<JmxApplication> apps = new HashSet();
383-
synchronized (unavailableApps) {
384-
apps.addAll(unavailableApps);
385-
unavailableApps.clear();
386-
}
387-
388-
cleanupUnavailableApps(apps);
389-
if (apps.isEmpty()) {
390-
heartbeatRunning = false;
391-
if (anotherHeartbeatPending) { // just a safe fallback, likely not needed at all
392-
anotherHeartbeatPending = false;
393-
scheduleHeartbeatImpl(false);
394-
} else {
395-
return;
396-
}
397-
}
398-
399-
final AtomicInteger counter = new AtomicInteger(apps.size());
400-
RequestProcessor processor = new RequestProcessor("JMX Heartbeat Processor", Math.min(counter.intValue(), HEARTBEAT_MAX_THREADS)); // NOI18N
401-
// System.err.println(">>> Heartbeat for " + counter + " targets at " + java.time.LocalTime.now());
402-
for (final JmxApplication app : apps) {
403-
processor.post(new Runnable() {
404-
@Override
405-
public void run() {
406-
try {
407-
boolean connected = false;
408-
try {
409-
ProxyClient client = new ProxyClient(app);
410-
client.connect();
411-
if (client.getConnectionState() == ConnectionState.CONNECTED) {
412-
app.setClient(client);
413-
connected = true;
414-
}
415-
} catch (IOException ex) {
416-
LOGGER.log(Level.FINE, "ProxyClient.connect", ex);
417-
}
418-
if (!connected) {
419-
synchronized (unavailableApps) { unavailableApps.add(app); }
420-
} else {
421-
app.setStateImpl(Stateful.STATE_AVAILABLE);
422-
423-
app.jmxModel = JmxModelFactory.getJmxModelFor(app);
424-
app.jvm = JvmFactory.getJVMFor(app);
359+
// TODO: move to JmxApplication implementation
360+
// PropertyChangeListener will be per JmxApplication instance - easy unregistering
361+
// introduce JmxApplication.disconnect()
362+
static boolean tryConnect(JmxApplication app) {
363+
try {
364+
ProxyClient client = new ProxyClient(app);
365+
client.connect();
366+
if (client.getConnectionState() == ConnectionState.CONNECTED) {
367+
app.setClient(client);
368+
369+
app.setStateImpl(Stateful.STATE_AVAILABLE);
425370

426-
app.jmxModel.addPropertyChangeListener(new PropertyChangeListener() {
427-
public void propertyChange(PropertyChangeEvent evt) {
428-
if (evt.getNewValue() == ConnectionState.CONNECTED) {
429-
app.setStateImpl(Stateful.STATE_AVAILABLE);
430-
} else {
431-
app.setStateImpl(Stateful.STATE_UNAVAILABLE);
432-
if (!app.isRemoved()) {
433-
synchronized (unavailableApps) { unavailableApps.add(app); }
434-
scheduleHeartbeatImpl(true);
435-
}
436-
// TODO: remove listener from model once not needed!
437-
}
438-
}
439-
});
440-
}
441-
} finally {
442-
if (counter.decrementAndGet() == 0) {
443-
boolean pendingApps;
444-
445-
synchronized (unavailableApps) {
446-
cleanupUnavailableApps(unavailableApps);
447-
pendingApps = !unavailableApps.isEmpty();
448-
heartbeatRunning = false;
449-
}
371+
app.jmxModel = JmxModelFactory.getJmxModelFor(app);
372+
app.jvm = JvmFactory.getJVMFor(app);
450373

451-
if (anotherHeartbeatPending || pendingApps) {
452-
anotherHeartbeatPending = false;
453-
scheduleHeartbeatImpl(false);
454-
}
374+
app.jmxModel.addPropertyChangeListener(new PropertyChangeListener() {
375+
public void propertyChange(PropertyChangeEvent evt) {
376+
if (evt.getNewValue() == ConnectionState.CONNECTED) {
377+
app.setStateImpl(Stateful.STATE_AVAILABLE);
378+
} else {
379+
app.setStateImpl(Stateful.STATE_UNAVAILABLE);
380+
if (!app.isRemoved()) JmxHeartbeat.scheduleLazily(app);
381+
// TODO: remove listener from model once not needed!
455382
}
456383
}
457-
}
458-
}, immediately ? HEARTBEAT_DELAY : HEARTBEAT_POLL_DELAY);
384+
});
385+
386+
return true;
387+
}
388+
} catch (IOException ex) {
389+
LOGGER.log(Level.FINE, "ProxyClient.connect", ex); // NOI18N
459390
}
391+
392+
return false;
460393
}
461394

462395

463-
private void cleanupUnavailableApps(Set<JmxApplication> apps) {
464-
String trueS = Boolean.TRUE.toString();
465-
Iterator<JmxApplication> appsI = apps.iterator();
466-
while (appsI.hasNext()) {
467-
JmxApplication app = appsI.next();
468-
if (app.isRemoved() || trueS.equals(app.getStorage().getCustomProperty(PROPERTY_DISABLE_HEARTBEAT)))
469-
appsI.remove();
470-
}
471-
}
472-
473-
474396
private void cleanupCreatedHost(Set<Host> hosts, Host host) {
475397
// NOTE: this is not absolutely failsafe, if resolving the JMX application
476398
// took a long time and its host has been added by the user/plugin, it may
@@ -573,8 +495,9 @@ public synchronized void dataChanged(DataChangeEvent<Host> event) {
573495
};
574496

575497
final AtomicInteger counter = new AtomicInteger(storageSetSize);
576-
RequestProcessor processor = new RequestProcessor("JMX Persistence Processor", Math.min(counter.intValue(), HEARTBEAT_MAX_THREADS)); // NOI18N
577-
498+
final Collection<JmxApplication> persistentApps = Collections.synchronizedList(new ArrayList());
499+
RequestProcessor processor = new RequestProcessor("JMX Persistence Processor", Math.min(storageSetSize, 10)); // NOI18N
500+
578501
for (final Storage storage : storageSet) {
579502
final String[] values = storage.getCustomProperties(keys);
580503
processor.post(new Runnable() {
@@ -589,8 +512,8 @@ public void run() {
589512
EnvironmentProvider ep = epid == null ? null :
590513
JmxConnectionSupportImpl.
591514
getProvider(epid);
592-
addLazyJmxApplication(false, null, values[0], values[2], values[3],
593-
values[1], ep, storage, values[5], false);
515+
persistentApps.add(addLazyJmxApplication(false, null, values[0], values[2], values[3],
516+
values[1], ep, storage, values[5], false));
594517
} catch (final JMXException e) {
595518
if (e.isConfig()) {
596519
DialogDisplayer.getDefault().notifyLater(
@@ -604,7 +527,8 @@ public void run() {
604527
failedAppsS.add(storage);
605528
}
606529
} finally {
607-
if (counter.decrementAndGet() == 0) scheduleHeartbeat();
530+
if (counter.decrementAndGet() == 0)
531+
JmxHeartbeat.scheduleImmediately(persistentApps.toArray(new JmxApplication[0]));
608532
}
609533
synchronized (persistedAppsCount) {
610534
persistedAppsCount[0]--;
@@ -686,10 +610,11 @@ public void run() {
686610
}
687611

688612

689-
private static class JMXException extends Exception {
613+
private static class JMXException extends Exception {
690614
private final boolean isConfig;
691615
public JMXException(boolean config, String message) { super(message); isConfig = config; }
692616
public JMXException(boolean config, String message, Throwable cause) { super(message,cause); isConfig = config; }
693617
public boolean isConfig() { return isConfig; }
694618
}
619+
695620
}

0 commit comments

Comments
 (0)