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

Optimize ObjId2FullSidMap::extractObjectId #599

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
27 changes: 17 additions & 10 deletions src/main/java/com/microsoft/jenkins/azuread/ObjId2FullSidMap.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
package com.microsoft.jenkins.azuread;

import java.util.HashMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class ObjId2FullSidMap extends HashMap<String, String> {

// full sid should be in the form of "<username> (<object_id>)".
private static final Pattern FULL_SID_PATTERN = Pattern.compile("(.*) \\((.*)\\)");

public void putFullSid(String fullSid) {
String objectId = extractObjectId(fullSid);
if (objectId != null) {
Expand All @@ -34,13 +29,25 @@ public String getOrOriginal(String objectId) {
}

static String extractObjectId(String fullSid) {
Matcher matcher = FULL_SID_PATTERN.matcher(fullSid);
if (matcher.matches()) {
String objectId = matcher.group(2);
return objectId;
} else {
// full sid should be in the form of "<username> (<object_id>)".

// this code previously used regex: (.*) \((.*)\), which was shown to be a CPU hotspot in certain
// Jenkins installations

if (fullSid.isEmpty()) {
return null;
}
if (fullSid.charAt(fullSid.length() - 1) != ')') {
return null;
}
int openingParenthesesPosition = fullSid.lastIndexOf('(');
if (openingParenthesesPosition <= 0) {
return null;
}
if (fullSid.charAt(openingParenthesesPosition - 1) != ' ') {
return null;
}
return fullSid.substring(openingParenthesesPosition + 1, fullSid.length() - 1);
}

static String generateFullSid(final String displayName, final String objectId) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package com.microsoft.jenkins.azuread;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;

public class ObjId2FullSidMapTest {

public static final String FULL_SID_1 = "Smith@test (00000000-00000000-00000000-00000000)";
Expand All @@ -29,4 +33,49 @@ public void testPutAndGet() {
Assert.assertEquals(FULL_SID_1, map.getOrOriginal(ObjId2FullSidMap.generateFullSid(NAME_1, OBJECT_ID_1)));
Assert.assertEquals("some string", map.getOrOriginal("some string"));
}

@Test
public void testExtractObjectId() {
Assert.assertNull(ObjId2FullSidMap.extractObjectId(""));
Assert.assertNull(ObjId2FullSidMap.extractObjectId("some string"));
Assert.assertNull(ObjId2FullSidMap.extractObjectId("email (id) "));
Assert.assertNull(ObjId2FullSidMap.extractObjectId("email (id"));
Assert.assertNull(ObjId2FullSidMap.extractObjectId("email id)"));
Assert.assertNull(ObjId2FullSidMap.extractObjectId("email(id)"));
Assert.assertEquals("id", ObjId2FullSidMap.extractObjectId("email (id)"));
Assert.assertEquals("", ObjId2FullSidMap.extractObjectId("email ()"));
Assert.assertEquals("id", ObjId2FullSidMap.extractObjectId(" (id)"));
}

@Ignore
@Test
public void testExtractObjectIdPerformance() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

FWIW JMH benchmarks are supported with something like this:
https://github.com/jenkinsci/junit-sql-storage-plugin/blob/master/src/test/java/io/jenkins/plugins/junit/storage/database/benchmarks/TrendGraphBenchmark.java

in case you are interested in converting but probably not worth it.

unsure if its worth committing this but certainly useful to provide it demonstrating the change.

final int numWarmupIterations = 1_000_000;
final int numExperiments = 1000;
final int numIterations = 10000;

final String fullSid = ObjId2FullSidMap.generateFullSid(EMAIL_1, OBJECT_ID_1);

// warmup
for (int i = 0; i < numWarmupIterations; i++) {
ObjId2FullSidMap.extractObjectId(fullSid);
}
// allow async JIT compilation to catch up
Thread.sleep(100);

List<Long> durationsNanos = new ArrayList<>();
for (int r = 0; r < numExperiments; r++) {
long start = System.nanoTime();
for (int i = 0; i < numIterations; i++) {
ObjId2FullSidMap.extractObjectId(fullSid);
}
long durationNanos = (System.nanoTime() - start) / numIterations;
durationsNanos.add(durationNanos);
}
durationsNanos.sort(null);
long median = durationsNanos.get(durationsNanos.size() / 2);

System.out.println("Median duration per call (nanos): " + median);
}

}
Loading