LuciferYang opened a new pull request, #12423:
URL: https://github.com/apache/gravitino/pull/12423
### What changes were proposed in this pull request?
`CredentialFactory.create` called `ServiceLoader.load(Credential.class)` on
every invocation and iterated/instantiated every registered `Credential`
implementation just to resolve one type -> class mapping. This caches the scan
once into an immutable `type -> class` map (initialization-on-demand holder
idiom) and turns `lookupCredential` into a single map get.
Lookup stays case-insensitive and still throws `No credential found for:
<type>` (preserving the caller's original casing) on a miss; duplicate-type
detection moves to cache-build time.
### Why are the changes needed?
`Credential` is a fixed, built-in SPI (custom credentials are added via
`CredentialProvider`, not by registering new `Credential` services), so the
`ServiceLoader` result is stable for the JVM lifetime and does not need to be
rescanned — nor all providers re-instantiated — on every lookup.
A JMH micro-benchmark shows the type lookup dropping from **~380 µs/op to
~64 ns/op**:
```
Benchmark Mode Cnt Score Error
Units
CredentialLookupBenchmark.newCachedLookup avgt 3 64.064 ± 8.449
ns/op
CredentialLookupBenchmark.oldPerCallScan avgt 3 380491.264 ± 35292.735
ns/op
```
<details>
<summary>Benchmark used (not part of this PR — drop under
<code>core/src/jmh/java/org/apache/gravitino/credential/</code> and run
<code>./gradlew :core:jmh</code>)</summary>
`oldPerCallScan` replays the previous `ServiceLoader.load` + stream-filter
per call; `newCachedLookup` does a single `get` on the pre-built map. Only the
type-lookup step is measured; the shared `newInstance` of the resolved
credential is excluded, since it is unchanged by this PR.
```java
package org.apache.gravitino.credential;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.ServiceLoader;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.infra.Blackhole;
@State(Scope.Benchmark)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class CredentialLookupBenchmark {
private Map<String, Class<? extends Credential>> cachedClasses;
private String[] types;
@Setup
public void setup() {
cachedClasses = new HashMap<>();
for (Credential credential : ServiceLoader.load(Credential.class)) {
cachedClasses.put(
credential.credentialType().toLowerCase(Locale.ROOT),
credential.getClass());
}
types = cachedClasses.keySet().toArray(new String[0]);
}
@Benchmark
public void oldPerCallScan(Blackhole bh) {
for (String type : types) {
bh.consume(oldLookup(type));
}
}
@Benchmark
public void newCachedLookup(Blackhole bh) {
for (String type : types) {
bh.consume(cachedClasses.get(type.toLowerCase(Locale.ROOT)));
}
}
private static Class<? extends Credential> oldLookup(String
credentialType) {
ServiceLoader<Credential> serviceLoader =
ServiceLoader.load(Credential.class);
List<Class<? extends Credential>> credentials =
StreamSupport.stream(serviceLoader.spliterator(), false)
.filter(credential ->
credentialType.equalsIgnoreCase(credential.credentialType()))
.map(Credential::getClass)
.collect(Collectors.toList());
return credentials.get(0);
}
}
```
</details>
### Does this PR introduce _any_ user-facing change?
No. Internal factory optimization; lookup semantics (case-insensitive
resolution, `No credential found for: <type>` on miss) are unchanged.
### How was this patch tested?
Existing `TestCredentialFactory` cases plus new ones for unknown-type
rejection (including that the error message preserves the original casing) and
case-insensitive lookup. `./gradlew :common:test --tests
"org.apache.gravitino.credential.TestCredentialFactory"` and spotless pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
