Skip to content

Commit

Permalink
change API for accessing management values to be purely string based
Browse files Browse the repository at this point in the history
  • Loading branch information
@dougxc
dougxc committed Apr 18, 2018
1 parent 45c5299 commit 153d8fb
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 84 deletions.
61 changes: 48 additions & 13 deletions ...spot.management/src/org/graalvm/compiler/hotspot/management/HotSpotGraalRuntimeMBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
package org.graalvm.compiler.hotspot.management;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;

import javax.management.Attribute;
import javax.management.AttributeList;
import javax.management.AttributeNotFoundException;
import javax.management.DynamicMBean;
import javax.management.InvalidAttributeValueException;
import javax.management.MBeanAttributeInfo;
import javax.management.MBeanException;
import javax.management.MBeanInfo;
Expand Down Expand Up @@ -74,35 +76,53 @@ HotSpotGraalRuntime getRuntime() {
return runtime;
}

private static final boolean DEBUG = Boolean.getBoolean(HotSpotGraalRuntimeMBean.class.getSimpleName() + ".debug");

@Override
public Object getAttribute(String name) throws AttributeNotFoundException {
Object[] result = runtime.getOptionValues(name);
if (result[0] == result) {
String[] result = runtime.getOptionValues(name);
String value = result[0];
if (value == null) {
throw new AttributeNotFoundException(name);
}
if (DEBUG) {
System.out.printf("getAttribute: %s = %s (type: %s)%n", name, value, value == null ? "null" : value.getClass().getName());
}
return result[0];
}

@SuppressFBWarnings(value = "ES_COMPARING_STRINGS_WITH_EQ", justification = "reference equality on the receiver is what we want")
@Override
public void setAttribute(Attribute attribute) throws AttributeNotFoundException {
public void setAttribute(Attribute attribute) throws AttributeNotFoundException, InvalidAttributeValueException {
String name = attribute.getName();
Object value = attribute.getValue();
String[] result = runtime.setOptionValues(new String[]{name}, new Object[]{value});
String svalue = String.valueOf(value);
if (DEBUG) {
System.out.printf("setAttribute: %s = %s (type: %s)%n", name, svalue, value == null ? "null" : value.getClass().getName());
}
String[] result = runtime.setOptionValues(new String[]{name}, new String[]{svalue});
if (result[0] != name) {
throw new AttributeNotFoundException(result[0]);
if (result[0] == null) {
throw new AttributeNotFoundException(name);
}
throw new InvalidAttributeValueException(result[0]);
}
}

@Override
public AttributeList getAttributes(String[] names) {
Object[] values = runtime.getOptionValues(names);
String[] values = runtime.getOptionValues(names);
AttributeList list = new AttributeList();
for (int i = 0; i < names.length; i++) {
if (values[i] == values) {
TTY.printf("No such option named %s%n", names[i]);
String value = values[i];
String name = names[i];
if (value == null) {
TTY.printf("No such option named %s%n", name);
} else {
list.add(new Attribute(names[i], values[i]));
if (DEBUG) {
System.out.printf("getAttributes: %s = %s (type: %s)%n", name, value, value == null ? "null" : value.getClass().getName());
}
list.add(new Attribute(name, value));
}
}
return list;
Expand All @@ -112,12 +132,18 @@ public AttributeList getAttributes(String[] names) {
@Override
public AttributeList setAttributes(AttributeList attributes) {
String[] names = new String[attributes.size()];
Object[] values = new Object[attributes.size()];
String[] values = new String[attributes.size()];

int i = 0;
for (Attribute attr : attributes.asList()) {
names[i] = attr.getName();
values[i] = attr.getValue();
String name = attr.getName();
names[i] = name;
Object value = attr.getValue();
String svalue = String.valueOf(value);
values[i] = svalue;
if (DEBUG) {
System.out.printf("setAttributes: %s = %s (type: %s)%n", name, svalue, value == null ? "null" : value.getClass().getName());
}
i++;
}
String[] result = runtime.setOptionValues(names, values);
Expand All @@ -126,6 +152,8 @@ public AttributeList setAttributes(AttributeList attributes) {
for (Attribute attr : attributes.asList()) {
if (names[i] == result[i]) {
setOk.add(attr);
} else if (result[i] == null) {
TTY.printf("Error setting %s to %s: unknown option%n", attr.getName(), attr.getValue());
} else {
TTY.printf("Error setting %s to %s: %s%n", attr.getName(), attr.getValue(), result[i]);
}
Expand All @@ -137,7 +165,14 @@ public AttributeList setAttributes(AttributeList attributes) {
@Override
public Object invoke(String actionName, Object[] params, String[] signature) throws MBeanException, ReflectionException {
try {
return runtime.invokeManagementAction(actionName, params);
if (DEBUG) {
System.out.printf("invoke: %s%s%n", actionName, Arrays.asList(params));
}
Object retvalue = runtime.invokeManagementAction(actionName, params);
if (DEBUG) {
System.out.printf("invoke: %s%s = %s%n", actionName, Arrays.asList(params), retvalue);
}
return retvalue;
} catch (Exception ex) {
throw new ReflectionException(ex);
}
Expand Down
144 changes: 107 additions & 37 deletions ...mpiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotGraalManagementTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,18 @@
import java.lang.management.ManagementFactory;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import javax.management.Attribute;
import javax.management.AttributeList;
import javax.management.AttributeNotFoundException;
import javax.management.InvalidAttributeValueException;
import javax.management.MBeanAttributeInfo;
import javax.management.MBeanInfo;
import javax.management.MBeanOperationInfo;
Expand All @@ -51,6 +56,7 @@
import org.graalvm.compiler.hotspot.HotSpotGraalManagementRegistration;
import org.graalvm.compiler.hotspot.HotSpotGraalRuntime;
import org.graalvm.compiler.options.EnumOptionKey;
import org.graalvm.compiler.options.NestedBooleanOptionKey;
import org.graalvm.compiler.options.OptionDescriptor;
import org.graalvm.compiler.options.OptionDescriptors;
import org.graalvm.compiler.options.OptionKey;
Expand All @@ -62,7 +68,7 @@

public class HotSpotGraalManagementTest {

private static final boolean DEBUG = Boolean.getBoolean(HotSpotGraalManagementTest.class.getSimpleName() + ".DEBUG");
private static final boolean DEBUG = Boolean.getBoolean(HotSpotGraalManagementTest.class.getSimpleName() + ".debug");

public HotSpotGraalManagementTest() {
try {
Expand Down Expand Up @@ -156,7 +162,7 @@ static class JunitShield {
* Tests changing the value of {@code option} via the management interface to a) a new legal
* value and b) an illegal value.
*/
private static void testOption(MBeanInfo mbeanInfo,
static void testOption(MBeanInfo mbeanInfo,
ObjectName mbeanName,
MBeanServer server,
HotSpotGraalRuntime runtime,
Expand All @@ -173,54 +179,85 @@ private static void testOption(MBeanInfo mbeanInfo,
MBeanAttributeInfo attrInfo = findAttributeInfo(name, mbeanInfo);
assertNotNull("Attribute not found for option " + name, attrInfo);

Object expectAttributeValue = currentValue;
if (optionKey instanceof EnumOptionKey) {
expectAttributeValue = String.valueOf(currentValue);
} else if (currentValue == null) {
expectAttributeValue = "null";
}
String expectAttributeValue = stringValue(currentValue, option.getOptionValueType() == String.class);
Object actualAttributeValue = server.getAttribute(mbeanName, name);
assertEquals(expectAttributeValue, actualAttributeValue);

Object newValue = null;
Object illegalValue = "illegalValue";
Map<String, String> legalValues = new HashMap<>();
List<String> illegalValues = new ArrayList<>();
if (optionKey instanceof EnumOptionKey) {
EnumOptionKey<?> enumOptionKey = (EnumOptionKey<?>) optionKey;
for (Object obj : enumOptionKey.getAllValues()) {
if (obj != currentValue) {
newValue = obj;
legalValues.put(obj.toString(), obj.toString());
}
}
illegalValues.add(String.valueOf(42));
} else if (optionType == Boolean.class) {
newValue = currentValue == null ? Boolean.TRUE : !((boolean) currentValue);
Object defaultValue;
if (optionKey instanceof NestedBooleanOptionKey) {
NestedBooleanOptionKey nbok = (NestedBooleanOptionKey) optionKey;
defaultValue = nbok.getMasterOption().getValue(runtime.getOptions());
} else {
defaultValue = optionKey.getDefaultValue();
}
legalValues.put("", unquotedStringValue(defaultValue));
illegalValues.add(String.valueOf(42));
illegalValues.add("true");
illegalValues.add("false");
} else if (optionType == String.class) {
newValue = currentValue + "Prime";
illegalValue = Integer.valueOf(42);
legalValues.put("", quotedStringValue(optionKey.getDefaultValue()));
legalValues.put("\"" + currentValue + "Prime\"", "\"" + currentValue + "Prime\"");
legalValues.put("\"quoted string\"", "\"quoted string\"");
illegalValues.add("\"unbalanced quotes");
illegalValues.add("\"");
illegalValues.add("non quoted string");
} else if (optionType == Float.class) {
newValue = currentValue == null ? 33F : ((float) currentValue) + 11F;
legalValues.put("", unquotedStringValue(optionKey.getDefaultValue()));
String value = unquotedStringValue(currentValue == null ? 33F : ((float) currentValue) + 11F);
legalValues.put(value, value);
illegalValues.add("string");
} else if (optionType == Double.class) {
newValue = currentValue == null ? 33D : ((double) currentValue) + 11D;
legalValues.put("", unquotedStringValue(optionKey.getDefaultValue()));
String value = unquotedStringValue(currentValue == null ? 33D : ((double) currentValue) + 11D);
legalValues.put(value, value);
illegalValues.add("string");
} else if (optionType == Integer.class) {
newValue = currentValue == null ? 33 : ((int) currentValue) + 11;
legalValues.put("", unquotedStringValue(optionKey.getDefaultValue()));
String value = unquotedStringValue(currentValue == null ? 33 : ((int) currentValue) + 11);
legalValues.put(value, value);
illegalValues.add("42.42");
illegalValues.add("string");
} else if (optionType == Long.class) {
newValue = currentValue == null ? 33L : ((long) currentValue) + 11L;
legalValues.put("", unquotedStringValue(optionKey.getDefaultValue()));
String value = unquotedStringValue(currentValue == null ? 33L : ((long) currentValue) + 11L);
legalValues.put(value, value);
illegalValues.add("42.42");
illegalValues.add("string");
}

if (DEBUG) {
System.out.printf("Changing %s from %s to %s%n", name, currentValue, newValue);
}
Attribute newAttributeValue = new Attribute(name, newValue);
newValues.add(newAttributeValue);
server.setAttribute(mbeanName, newAttributeValue);
Attribute originalAttributeValue = new Attribute(name, currentValue);
Attribute originalAttributeValue = new Attribute(name, expectAttributeValue);
try {
Object actual = optionKey.getValue(runtime.getOptions());
assertEquals(newValue, actual);
actual = server.getAttribute(mbeanName, name);
if (optionKey instanceof EnumOptionKey) {
newValue = String.valueOf(newValue);
for (Map.Entry<String, String> e : legalValues.entrySet()) {
String legalValue = e.getKey();
if (DEBUG) {
System.out.printf("Changing %s from %s to %s%n", name, currentValue, legalValue);
}
Attribute newAttributeValue = new Attribute(name, legalValue);
newValues.add(newAttributeValue);
server.setAttribute(mbeanName, newAttributeValue);
Object actual = optionKey.getValue(runtime.getOptions());
actual = server.getAttribute(mbeanName, name);
String expectValue = e.getValue();
if (option.getOptionValueType() == String.class && expectValue == null) {
expectValue = "";
} else if (option.getOptionKey() instanceof NestedBooleanOptionKey && null == expectValue) {
NestedBooleanOptionKey nbok = (NestedBooleanOptionKey) option.getOptionKey();
expectValue = String.valueOf(nbok.getValue(runtime.getOptions()));
actual = server.getAttribute(mbeanName, name);
}
assertEquals(expectValue, actual);
}
assertEquals(newValue, actual);
} finally {
if (DEBUG) {
System.out.printf("Resetting %s to %s%n", name, currentValue);
Expand All @@ -230,19 +267,30 @@ private static void testOption(MBeanInfo mbeanInfo,
}

try {
if (DEBUG) {
System.out.printf("Changing %s from %s to illegal value %s%n", name, currentValue, illegalValue);
for (Object illegalValue : illegalValues) {
if (DEBUG) {
System.out.printf("Changing %s from %s to illegal value %s%n", name, currentValue, illegalValue);
}
server.setAttribute(mbeanName, new Attribute(name, illegalValue));
Assert.fail("Expected setting " + name + " to " + illegalValue + " to fail");
}
server.setAttribute(mbeanName, new Attribute(name, illegalValue));
Assert.fail("Expected setting " + name + " to " + illegalValue + " to fail");
} catch (Exception e) {
} catch (InvalidAttributeValueException e) {
// Expected
} finally {
if (DEBUG) {
System.out.printf("Resetting %s to %s%n", name, currentValue);
}
server.setAttribute(mbeanName, originalAttributeValue);
}

try {

String unknownOptionName = "definitely not an option name";
server.setAttribute(mbeanName, new Attribute(unknownOptionName, ""));
Assert.fail("Expected setting option with name \"" + unknownOptionName + "\" to fail");
} catch (AttributeNotFoundException e) {
// Expected
}
}

static MBeanAttributeInfo findAttributeInfo(String attrName, MBeanInfo info) {
Expand All @@ -257,6 +305,28 @@ static MBeanAttributeInfo findAttributeInfo(String attrName, MBeanInfo info) {
}
}

private static String quotedStringValue(Object optionValue) {
return stringValue(optionValue, true);
}

private static String unquotedStringValue(Object optionValue) {
return stringValue(optionValue, false);
}

private static String stringValue(Object optionValue, boolean withQuoting) {
if (optionValue == null) {
return "";
}
if (withQuoting) {
return "\"" + optionValue + "\"";
}
return String.valueOf(optionValue);
}

private static String quoted(Object s) {
return "\"" + s + "\"";
}

@Test
public void dumpOperation() throws Exception {
assertNotNull("Server is started", ManagementFactory.getPlatformMBeanServer());
Expand Down Expand Up @@ -297,7 +367,7 @@ public void dumpOperation() throws Exception {
Object originalShowDumpFiles = server.getAttribute(mbeanName, showDumpFiles.getName());
final File tmpDir = new File(HotSpotGraalManagementTest.class.getSimpleName() + "_" + System.currentTimeMillis()).getAbsoluteFile();

server.setAttribute(mbeanName, new Attribute(dumpPath.getName(), tmpDir.toString()));
server.setAttribute(mbeanName, new Attribute(dumpPath.getName(), quoted(tmpDir)));
// Force output to a file even if there's a running IGV instance available.
server.setAttribute(mbeanName, new Attribute(printGraphFile.getName(), true));
server.setAttribute(mbeanName, new Attribute(showDumpFiles.getName(), false));
Expand Down
Loading

0 comments on commit 153d8fb

Please sign in to comment.