Allow method inlining of ObjectUtils.nullSafeEquals() [SPR-14349]

[spring-projects-issues]

Christoph Dreis opened SPR-14349 and commented

Hey,

while doing a benchmark in our project with the JVM options

-XX:+PrintCompilation -XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining

I noticed that ObjectUtils.nullSafeEquals() is a hot method, but is unfortunately too big for the VM to inline it. At least with the defaults of 325 bytecodes as it currently shows up with 337 bytes.

...
 @ 8   org.springframework.util.ObjectUtils::nullSafeEquals (337 bytes)   hot method too big
...

I made a small improvement by simply extracting the more uncommon use-case of checking for array equality, which reduces the method to 55 bytes and allows the inlining of the method. As a small side-benefit it also reduces the cyclomatic complexity of the method itself.

...
@ 8   com.dreis.benchmark.ObjectUtilsInlinedEquals::nullSafeEquals (55 bytes)   inline (hot)
...

I also created a small microbenchmark that looks like this:

@BenchmarkMode(Mode.Throughput)
@State(Scope.Thread)
public class ObjectUtilsBenchmark {

	@State(Scope.Thread)
	public static class TestState {
		public String[] firstStringArray = new String[] {"a", "b", "c"};
		public String[] secondStringArray = new String[] {"a", "b", "c"};
		public List<String> firstStringCollection = Arrays.asList("a", "b", "c");
		public List<String> secondStringCollection = Arrays.asList("a", "b", "c");
	}

	@Benchmark
	public boolean testStringArray(TestState testState) {
		return ObjectUtils.nullSafeEquals(testState.firstStringArray, testState.secondStringArray);
	}

	@Benchmark
	public boolean testStringCollection(TestState testState) {
		return ObjectUtils.nullSafeEquals(testState.firstStringCollection, testState.secondStringCollection);
	}

	@Benchmark
	public boolean testStringArrayInlined(TestState testState) {
		return ObjectUtilsInlinedEquals.nullSafeEquals(testState.firstStringArray, testState.secondStringArray);
	}

	@Benchmark
	public boolean testStringCollectionInlined(TestState testState) {
		return ObjectUtilsInlinedEquals.nullSafeEquals(testState.firstStringCollection, testState.secondStringCollection);
	}

}

The results of this microbenchmark look roughly like this.

Benchmark                                          Mode  Cnt          Score        Error  Units
ObjectUtilsBenchmark.testStringArray              thrpt   20  127900470,357 ± 985253,526  ops/s
ObjectUtilsBenchmark.testStringArrayInlined       thrpt   20  149902422,075 ± 905479,738  ops/s
ObjectUtilsBenchmark.testStringCollection         thrpt   20   61584045,911 ± 345039,219  ops/s
ObjectUtilsBenchmark.testStringCollectionInlined  thrpt   20   70381357,642 ± 751583,252  ops/s

I created a PR with my changes at #1076 and would be very happy if this gets accepted.

Keep up the great work!

Cheers,
Christoph

---

Affects: 3.2.17, 4.2.6

Referenced from: pull request #1076, and commits ca12e13, 14ab980, 71df9ce

Backported to: 4.2.7, 3.2.18

[spring-projects-issues]

Juergen Hoeller commented

Good catch, and perfect timing! I've rolled a slightly revised variant into master for release in 4.3 GA tomorrow...

[spring-projects-issues]

Juergen Hoeller commented

Being at it, this can also easily be backported to 4.2.7, so this change will be available in the upcoming 4.3.0.BUILD-SNAPSHOT as well as 4.2.7.BUILD-SNAPSHOT now.

[spring-projects-issues]

Christoph Dreis commented

Thanks for the quick feedback, @Juergen Hoeller ! Of course, I was hoping to get the contribution to be honest, but I'm really glad the actual issue is solved that quickly and even in the upcoming release tomorrow. Thanks again! Should I close the PR then?

[spring-projects-issues]

Christoph Dreis commented

Nevermind, you closed it already ;-)