Skip to content

Commit

Permalink
8299234: JMX Repository.query performance
Browse files Browse the repository at this point in the history 
Reviewed-by: dfuchs, kevinw
  • Loading branch information
Alexey Bakhtin committed Feb 20, 2023
1 parent e47e9ec commit 9a79722
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 128 deletions.
135 changes: 10 additions & 125 deletions src/java.management/share/classes/com/sun/jmx/mbeanserver/Repository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,131 +106,18 @@ public interface RegistrationContext {
// Private fields <=============================================

// Private methods --------------------------------------------->

/* This class is used to match an ObjectName against a pattern. */
private static final class ObjectNamePattern {
private final String[] keys;
private final String[] values;
private final String properties;
private final boolean isPropertyListPattern;
private final boolean isPropertyValuePattern;

/**
* The ObjectName pattern against which ObjectNames are matched.
**/
public final ObjectName pattern;

/**
* Builds a new ObjectNamePattern object from an ObjectName pattern.
* @param pattern The ObjectName pattern under examination.
**/
public ObjectNamePattern(ObjectName pattern) {
this(pattern.isPropertyListPattern(),
pattern.isPropertyValuePattern(),
pattern.getCanonicalKeyPropertyListString(),
pattern.getKeyPropertyList(),
pattern);
}

/**
* Builds a new ObjectNamePattern object from an ObjectName pattern
* constituents.
* @param propertyListPattern pattern.isPropertyListPattern().
* @param propertyValuePattern pattern.isPropertyValuePattern().
* @param canonicalProps pattern.getCanonicalKeyPropertyListString().
* @param keyPropertyList pattern.getKeyPropertyList().
* @param pattern The ObjectName pattern under examination.
**/
ObjectNamePattern(boolean propertyListPattern,
boolean propertyValuePattern,
String canonicalProps,
Map<String,String> keyPropertyList,
ObjectName pattern) {
this.isPropertyListPattern = propertyListPattern;
this.isPropertyValuePattern = propertyValuePattern;
this.properties = canonicalProps;
final int len = keyPropertyList.size();
this.keys = new String[len];
this.values = new String[len];
int i = 0;
for (Map.Entry<String,String> entry : keyPropertyList.entrySet()) {
keys[i] = entry.getKey();
values[i] = entry.getValue();
i++;
}
this.pattern = pattern;
}

/**
* Return true if the given ObjectName matches the ObjectName pattern
* for which this object has been built.
* WARNING: domain name is not considered here because it is supposed
* not to be wildcard when called. PropertyList is also
* supposed not to be zero-length.
* @param name The ObjectName we want to match against the pattern.
* @return true if <code>name</code> matches the pattern.
**/
public boolean matchKeys(ObjectName name) {
// If key property value pattern but not key property list
// pattern, then the number of key properties must be equal
//
if (isPropertyValuePattern &&
!isPropertyListPattern &&
(name.getKeyPropertyList().size() != keys.length))
return false;

// If key property value pattern or key property list pattern,
// then every property inside pattern should exist in name
//
if (isPropertyValuePattern || isPropertyListPattern) {
for (int i = keys.length - 1; i >= 0 ; i--) {
// Find value in given object name for key at current
// index in receiver
//
String v = name.getKeyProperty(keys[i]);
// Did we find a value for this key ?
//
if (v == null) return false;
// If this property is ok (same key, same value), go to next
//
if (isPropertyValuePattern &&
pattern.isPropertyValuePattern(keys[i])) {
// wildmatch key property values
// values[i] is the pattern;
// v is the string
if (Util.wildmatch(v,values[i]))
continue;
else
return false;
}
if (v.equals(values[i])) continue;
return false;
}
return true;
}

// If no pattern, then canonical names must be equal
//
final String p1 = name.getCanonicalKeyPropertyListString();
final String p2 = properties;
return (p1.equals(p2));
}
}

/**
* Add all the matching objects from the given hashtable in the
* result set for the given ObjectNamePattern
* Do not check whether the domains match (only check for matching
* key property lists - see <i>matchKeys()</i>)
* result set for the given pattern
**/
private void addAllMatching(final Map<String,NamedObject> moiTb,
final Set<NamedObject> result,
final ObjectNamePattern pattern) {
final ObjectName pattern) {
synchronized (moiTb) {
for (NamedObject no : moiTb.values()) {
final ObjectName on = no.getName();
// if all couples (property, value) are contained
if (pattern.matchKeys(on)) result.add(no);
if (pattern.apply(ObjectName.getInstance(on))) result.add(no);
}
}
}
Expand Down Expand Up @@ -519,7 +406,7 @@ public Set<NamedObject> query(ObjectName pattern, QueryExp query) {
pattern.getCanonicalName().length() == 0 ||
pattern.equals(ObjectName.WILDCARD))
name = ObjectName.WILDCARD;
else name = pattern;
else name = ObjectName.getInstance(pattern);

lock.readLock().lock();
try {
Expand All @@ -543,38 +430,36 @@ public Set<NamedObject> query(ObjectName pattern, QueryExp query) {
name.getCanonicalKeyPropertyListString();
final boolean allNames =
(canonical_key_property_list_string.length()==0);
final ObjectNamePattern namePattern =
(allNames?null:new ObjectNamePattern(name));
final String dom2Match = name.getDomain();

// All names in default domain
if (name.getDomain().length() == 0) {
if (dom2Match.length() == 0) {
final Map<String,NamedObject> moiTb = domainTb.get(domain);
if (allNames)
result.addAll(moiTb.values());
else
addAllMatching(moiTb, result, namePattern);
addAllMatching(moiTb, result, Util.newObjectName(domain + name.getCanonicalName()));
return result;
}

if (!name.isDomainPattern()) {
final Map<String,NamedObject> moiTb = domainTb.get(name.getDomain());
final Map<String,NamedObject> moiTb = domainTb.get(dom2Match);
if (moiTb == null) return Collections.emptySet();
if (allNames)
result.addAll(moiTb.values());
else
addAllMatching(moiTb, result, namePattern);
addAllMatching(moiTb, result, name);
return result;
}

// Pattern matching in the domain name (*, ?)
final String dom2Match = name.getDomain();
for (String dom : domainTb.keySet()) {
if (Util.wildmatch(dom, dom2Match)) {
final Map<String,NamedObject> moiTb = domainTb.get(dom);
if (allNames)
result.addAll(moiTb.values());
else
addAllMatching(moiTb, result, namePattern);
addAllMatching(moiTb, result, name);
}
}
return result;
Expand Down
2 changes: 1 addition & 1 deletion src/java.management/share/classes/com/sun/jmx/mbeanserver/Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public static int hashCode(String[] names, Object[] values) {
of the string, which will match if that remainder looks like
YC, so the whole string looks like AXBYC.
*/
private static boolean wildmatch(final String str, final String pat,
public static boolean wildmatch(final String str, final String pat,
int stri, final int strend, int pati, final int patend) {

// System.out.println("matching "+pat.substring(pati,patend)+
Expand Down
11 changes: 11 additions & 0 deletions src/java.management/share/classes/javax/management/ObjectName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2016,12 +2016,23 @@ public boolean apply(ObjectName name) {
}

private final boolean matchDomains(ObjectName name) {
boolean useOptimized = (this.getClass() == ObjectName.class) &&
(name.getClass() == ObjectName.class);

if (isDomainPattern()) {
// wildmatch domains
// This ObjectName is the pattern
// The other ObjectName is the string.
if (useOptimized) {
return Util.wildmatch(name._canonicalName, _canonicalName,
0, name.getDomainLength(), 0, getDomainLength());
}
return Util.wildmatch(name.getDomain(),getDomain());
}
if (useOptimized) {
return getDomainLength() == name.getDomainLength() &&
_canonicalName.regionMatches(0, name._canonicalName, 0, getDomainLength());
}
return getDomain().equals(name.getDomain());
}

Expand Down
12 changes: 10 additions & 2 deletions test/jdk/javax/management/ObjectName/ApplyWildcardTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -23,7 +23,7 @@

/*
* @test
* @bug 4716807
* @bug 4716807 8299234
* @summary Test the ObjectName.apply(ObjectName) method
* with wildcards in the key properties value part.
* @author Luis-Miguel Alventosa
Expand Down Expand Up @@ -75,6 +75,10 @@ public class ApplyWildcardTest {
{ "d:k1=\"a?b\",k2=\"c*d\"", "d:k1=\"axb\",k2=\"cyzd\"" },
{ "d:k1=\"a?b\",k2=\"c*d\",*", "d:k1=\"axb\",k2=\"cyzd\",k3=\"v3\"" },
{ "d:*,k1=\"a?b\",k2=\"c*d\"", "d:k1=\"axb\",k2=\"cyzd\",k3=\"v3\"" },

{ "*:k=a", "d:k=a" },
{ "*b*:k=a", "abc:k=a" },
{ "?b?:k=a", "abc:k=a" },
};

private static final String negativeTests[][] = {
Expand Down Expand Up @@ -115,6 +119,10 @@ public class ApplyWildcardTest {
{ "d:k1=\"a?b\",k2=\"c*d\"", "d:k1=\"ab\",k2=\"cd\"" },
{ "d:k1=\"a?b\",k2=\"c*d\",*", "d:k1=\"ab\",k2=\"cd\",k3=\"v3\"" },
{ "d:*,k1=\"a?b\",k2=\"c*d\"", "d:k1=\"ab\",k2=\"cd\",k3=\"v3\"" },

{ "?:k=a", "dd:k=a" },
{ "*b*:k=a", "adc:k=a" },
{ "?b?:k=a", "adc:k=a" },
};

private static int runPositiveTests() {
Expand Down

1 comment on commit 9a79722

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

Please sign in to comment.