Skip to content

Commit

Permalink
Use Map.of instead of custom implementation (#9032)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dstepanov
dstepanov authored Mar 31, 2023
1 parent 2eb7b23 commit d65a608
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 152 deletions.
3 changes: 1 addition & 2 deletions benchmarks/src/jmh/java/io/micronaut/http/server/stack/FullHttpStackBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;
import org.openjdk.jmh.profile.AsyncProfiler;
import org.openjdk.jmh.runner.Runner;
import org.openjdk.jmh.runner.options.Options;
import org.openjdk.jmh.runner.options.OptionsBuilder;
Expand Down Expand Up @@ -67,7 +66,7 @@ public static void main(String[] args) throws Exception {
.measurementIterations(30)
.mode(Mode.AverageTime)
.timeUnit(TimeUnit.NANOSECONDS)
.addProfiler(AsyncProfiler.class, "libPath=/home/yawkat/bin/async-profiler-2.9-linux-x64/build/libasyncProfiler.so;output=flamegraph")
// .addProfiler(AsyncProfiler.class, "libPath=/home/yawkat/bin/async-profiler-2.9-linux-x64/build/libasyncProfiler.so;output=flamegraph")
.forks(1)
.jvmArgsAppend("-Djmh.executor=CUSTOM", "-Djmh.executor.class=" + JmhFastThreadLocalExecutor.class.getName())
.build();
Expand Down
100 changes: 52 additions & 48 deletions core-processor/src/main/java/io/micronaut/inject/annotation/AnnotationMetadataWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,17 @@
*/
package io.micronaut.inject.annotation;

import io.micronaut.context.expressions.AbstractEvaluatedExpression;
import io.micronaut.core.annotation.AnnotationClassValue;
import io.micronaut.core.annotation.AnnotationMetadata;
import io.micronaut.core.annotation.AnnotationMetadataDelegate;
import io.micronaut.core.annotation.AnnotationUtil;
import io.micronaut.core.expressions.EvaluatedExpressionReference;
import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.UsedByGeneratedCode;
import io.micronaut.core.expressions.EvaluatedExpressionReference;
import io.micronaut.core.reflect.ReflectionUtils;
import io.micronaut.core.util.ArrayUtils;
import io.micronaut.core.util.CollectionUtils;
import io.micronaut.inject.ast.ClassElement;
import io.micronaut.context.expressions.AbstractEvaluatedExpression;
import io.micronaut.inject.writer.AbstractAnnotationMetadataWriter;
import io.micronaut.inject.writer.AbstractClassFileWriter;
import io.micronaut.inject.writer.ClassGenerationException;
Expand All @@ -42,16 +41,13 @@
import java.io.IOException;
import java.io.OutputStream;
import java.lang.reflect.Array;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Responsible for writing class files that are instances of {@link AnnotationMetadata}.
Expand All @@ -66,14 +62,6 @@ public class AnnotationMetadataWriter extends AbstractClassFileWriter {
private static final Type TYPE_DEFAULT_ANNOTATION_METADATA_HIERARCHY = Type.getType(AnnotationMetadataHierarchy.class);
private static final Type TYPE_ANNOTATION_CLASS_VALUE = Type.getType(AnnotationClassValue.class);

private static final org.objectweb.asm.commons.Method METHOD_LIST_OF = Method.getMethod(
ReflectionUtils.getRequiredInternalMethod(
AnnotationUtil.class,
"internListOf",
Object[].class
)
);

private static final org.objectweb.asm.commons.Method METHOD_REGISTER_ANNOTATION_DEFAULTS = Method.getMethod(
ReflectionUtils.getRequiredInternalMethod(
DefaultAnnotationMetadata.class,
Expand Down Expand Up @@ -163,10 +151,6 @@ public class AnnotationMetadataWriter extends AbstractClassFileWriter {
Object.class
));

private static final Type ANNOTATION_UTIL_TYPE = Type.getType(AnnotationUtil.class);
private static final Type LIST_TYPE = Type.getType(List.class);
private static final String EMPTY_LIST = "EMPTY_LIST";

private static final String LOAD_CLASS_PREFIX = "$micronaut_load_class_value_";

private final String className;
Expand Down Expand Up @@ -436,27 +420,6 @@ public static void writeAnnotationDefaults(
}
}

private static void pushListOfString(GeneratorAdapter methodVisitor, List<String> names) {
if (names != null) {
names = names.stream().filter(Objects::nonNull).collect(Collectors.toList());
}
if (names == null || names.isEmpty()) {
methodVisitor.getStatic(Type.getType(Collections.class), EMPTY_LIST, LIST_TYPE);
return;
}
int totalSize = names.size();
// start a new array
pushNewArray(methodVisitor, Object.class, totalSize);
int i = 0;
for (String name : names) {
// use the property name as the key
pushStoreStringInArray(methodVisitor, i++, totalSize, name);
// use the property type as the value
}
// invoke the AbstractBeanDefinition.createMap method
methodVisitor.invokeStatic(ANNOTATION_UTIL_TYPE, METHOD_LIST_OF);
}

private static void instantiateInternal(
Type owningType,
ClassWriter declaringClassWriter,
Expand Down Expand Up @@ -569,8 +532,7 @@ private static void pushValue(Type declaringType, ClassVisitor declaringClassWri
}
} else if (value instanceof String) {
methodVisitor.push(value.toString());
} else if (value instanceof AnnotationClassValue) {
AnnotationClassValue acv = (AnnotationClassValue) value;
} else if (value instanceof AnnotationClassValue<?> acv) {
if (acv.isInstantiated()) {
methodVisitor.visitTypeInsn(NEW, TYPE_ANNOTATION_CLASS_VALUE.getInternalName());
methodVisitor.visitInsn(DUP);
Expand Down Expand Up @@ -600,11 +562,11 @@ private static void pushValue(Type declaringType, ClassVisitor declaringClassWri
);
}
}
} else if (value instanceof Collection) {
if (((Collection<?>) value).isEmpty()) {
} else if (value instanceof Collection<?> collection) {
if (collection.isEmpty()) {
pushEmptyObjectsArray(methodVisitor);
} else {
List array = Arrays.asList(((Collection) value).toArray());
List array = CollectionUtils.iterableToList(collection);
int len = array.size();
boolean first = true;
Class<?> arrayType = Object.class;
Expand Down Expand Up @@ -657,8 +619,7 @@ private static void pushValue(Type declaringType, ClassVisitor declaringClassWri
if (boxValue) {
pushBoxPrimitiveIfNecessary(ReflectionUtils.getPrimitiveType(value.getClass()), methodVisitor);
}
} else if (value instanceof io.micronaut.core.annotation.AnnotationValue) {
io.micronaut.core.annotation.AnnotationValue data = (io.micronaut.core.annotation.AnnotationValue) value;
} else if (value instanceof io.micronaut.core.annotation.AnnotationValue<?> data) {
String annotationName = data.getAnnotationName();
Map<CharSequence, Object> values = data.getValues();
Type annotationValueType = Type.getType(io.micronaut.core.annotation.AnnotationValue.class);
Expand Down Expand Up @@ -698,13 +659,56 @@ private static void pushValue(Type declaringType, ClassVisitor declaringClassWri
() -> pushValue(declaringType, declaringClassWriter, methodVisitor, v,
defaultsStorage, loadTypeMethods, false));
}
} else {
throw new IllegalStateException();
}

methodVisitor.invokeConstructor(type, CONSTRUCTOR_CONTEXT_EVALUATED_EXPRESSION);

} else {
methodVisitor.visitInsn(ACONST_NULL);
throw new IllegalStateException("Unsupported Map value: " + value + " " + value.getClass().getName());
}
}

public static boolean isSupportedMapValue(Object value) {
if (value == null) {
return false;
} else if (value instanceof Boolean) {
return true;
} else if (value instanceof String) {
return true;
} else if (value instanceof AnnotationClassValue<?>) {
return true;
} else if (value instanceof Enum<?>) {
return true;
} else if (value.getClass().isArray()) {
return true;
} else if (value instanceof Collection<?>) {
return true;
} else if (value instanceof Map) {
return true;
} else if (value instanceof Long) {
return true;
} else if (value instanceof Double) {
return true;
} else if (value instanceof Float) {
return true;
} else if (value instanceof Byte) {
return true;
} else if (value instanceof Short) {
return true;
} else if (value instanceof Character) {
return true;
} else if (value instanceof Number) {
return true;
} else if (value instanceof io.micronaut.core.annotation.AnnotationValue<?>) {
return true;
} else if (value instanceof EvaluatedExpressionReference) {
return true;
} else if (value instanceof Class<?>) {
// The class should be added as AnnotationClassValue
return false;
}
return false;
}

private static void pushEmptyObjectsArray(GeneratorAdapter methodVisitor) {
Expand Down
106 changes: 70 additions & 36 deletions core-processor/src/main/java/io/micronaut/inject/writer/AbstractClassFileWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package io.micronaut.inject.writer;

import io.micronaut.core.annotation.AnnotationMetadata;
import io.micronaut.core.annotation.AnnotationUtil;
import io.micronaut.core.annotation.Generated;
import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.NonNull;
Expand Down Expand Up @@ -55,9 +54,9 @@
import java.util.AbstractMap;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand All @@ -70,6 +69,8 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static io.micronaut.inject.annotation.AnnotationMetadataWriter.isSupportedMapValue;

/**
* Abstract class that writes generated classes to disk and provides convenience methods for building classes.
*
Expand Down Expand Up @@ -157,34 +158,40 @@ public abstract class AbstractClassFileWriter implements Opcodes, OriginatingEle
)
);

private static final Type ANNOTATION_UTIL_TYPE = Type.getType(AnnotationUtil.class);
private static final Type MAP_TYPE = Type.getType(Map.class);
private static final String EMPTY_MAP = "EMPTY_MAP";
private static final Type LIST_TYPE = Type.getType(List.class);

private static final org.objectweb.asm.commons.Method[] MAP_OF;
private static final org.objectweb.asm.commons.Method[] LIST_OF;
private static final org.objectweb.asm.commons.Method MAP_BY_ARRAY;
private static final org.objectweb.asm.commons.Method MAP_ENTRY;
private static final org.objectweb.asm.commons.Method LIST_BY_ARRAY;

static {
MAP_OF = new Method[11];
for (int i = 1; i < MAP_OF.length; i++) {
for (int i = 0; i < MAP_OF.length; i++) {
Class[] mapArgs = new Class[i * 2];
for (int k = 0; k < i * 2; k += 2) {
mapArgs[k] = String.class;
mapArgs[k] = Object.class;
mapArgs[k + 1] = Object.class;
}
MAP_OF[i] = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(AnnotationUtil.class, "mapOf", mapArgs));
MAP_OF[i] = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(Map.class, "of", mapArgs));
}
MAP_BY_ARRAY = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(AnnotationUtil.class, "mapOf", Object[].class));
MAP_ENTRY = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(Map.class, "entry", Object.class, Object.class));
MAP_BY_ARRAY = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(Map.class, "ofEntries", Map.Entry[].class));
}

private static final org.objectweb.asm.commons.Method INTERN_MAP_OF_METHOD = org.objectweb.asm.commons.Method.getMethod(
ReflectionUtils.getRequiredInternalMethod(
AnnotationUtil.class,
"internMapOf",
String.class,
Object.class
)
);
static {
LIST_OF = new Method[11];
for (int i = 0; i < LIST_OF.length; i++) {
Class[] listArgs = new Class[i];
for (int k = 0; k < i; k += 1) {
listArgs[k] = Object.class;
}
LIST_OF[i] = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(List.class, "of", listArgs));
}
LIST_BY_ARRAY = org.objectweb.asm.commons.Method.getMethod(ReflectionUtils.getRequiredMethod(List.class, "of", Object[].class));
}

protected final OriginatingElements originatingElements;

Expand Down Expand Up @@ -235,7 +242,7 @@ protected static void pushTypeArgumentElements(
Map<String, Integer> defaults,
Map<String, GeneratorAdapter> loadTypeMethods) {
if (types == null || types.isEmpty()) {
generatorAdapter.visitInsn(ACONST_NULL);
pushNewArray(generatorAdapter, Argument.class, 0);
return;
}
pushTypeArgumentElements(annotationMetadataWithDefaults, owningType, owningTypeWriter, generatorAdapter, declaringElementName, null, types, new HashSet<>(5), defaults, loadTypeMethods);
Expand Down Expand Up @@ -1798,44 +1805,71 @@ ClassElement invokeMethod(@NonNull GeneratorAdapter generatorAdapter, @NonNull M
return returnType;
}

public static <T> void pushStringMapOf(GeneratorAdapter generatorAdapter, Map<? extends CharSequence, T> annotationData,
public static <T> void pushStringMapOf(GeneratorAdapter generatorAdapter,
Map<? extends CharSequence, T> annotationData,
boolean skipEmpty,
T empty,
Consumer<T> pushValue) {
Set<? extends Map.Entry<String, T>> entrySet = annotationData != null ? annotationData.entrySet()
.stream()
.filter(e -> !skipEmpty || (e.getKey() != null && e.getValue() != null))
.filter(e -> !skipEmpty || (e.getKey() != null && isSupportedMapValue(e.getValue())))
.map(e -> e.getValue() == null && empty != null ? new AbstractMap.SimpleEntry<>(e.getKey().toString(), empty) : new AbstractMap.SimpleEntry<>(e.getKey().toString(), e.getValue()))
.collect(Collectors.toCollection(() -> new TreeSet<>(Map.Entry.comparingByKey()))) : null;
if (entrySet == null || entrySet.isEmpty()) {
generatorAdapter.getStatic(Type.getType(Collections.class), EMPTY_MAP, MAP_TYPE);
invokeInterfaceStatic(generatorAdapter, MAP_TYPE, MAP_OF[0]);
return;
}
if (entrySet.size() == 1 && entrySet.iterator().next().getValue() == Collections.EMPTY_MAP) {
if (entrySet.size() < MAP_OF.length) {
for (Map.Entry<String, T> entry : entrySet) {
generatorAdapter.push(entry.getKey());
pushValue.accept(entry.getValue());
}
generatorAdapter.invokeStatic(ANNOTATION_UTIL_TYPE, INTERN_MAP_OF_METHOD);
} else if (entrySet.size() < MAP_OF.length) {
for (Map.Entry<String, T> entry : entrySet) {
generatorAdapter.push(entry.getKey());
pushValue.accept(entry.getValue());
}
generatorAdapter.invokeStatic(ANNOTATION_UTIL_TYPE, MAP_OF[entrySet.size()]);
invokeInterfaceStatic(generatorAdapter, MAP_TYPE, MAP_OF[entrySet.size()]);
} else {
int totalSize = entrySet.size() * 2;
int totalSize = entrySet.size();
// start a new array
pushNewArray(generatorAdapter, Object.class, totalSize);
pushNewArray(generatorAdapter, Map.Entry.class, totalSize);
int i = 0;
for (Map.Entry<? extends CharSequence, T> entry : entrySet) {
// use the property name as the key
String memberName = entry.getKey().toString();
pushStoreStringInArray(generatorAdapter, i++, totalSize, memberName);
// use the property type as the value
pushStoreInArray(generatorAdapter, i++, totalSize, () -> pushValue.accept(entry.getValue()));

pushStoreInArray(generatorAdapter, i++, totalSize, () -> {
generatorAdapter.push(entry.getKey().toString());
pushValue.accept(entry.getValue());
invokeInterfaceStatic(generatorAdapter, MAP_TYPE, MAP_ENTRY);
});

}
generatorAdapter.invokeStatic(ANNOTATION_UTIL_TYPE, MAP_BY_ARRAY);
invokeInterfaceStatic(generatorAdapter, MAP_TYPE, MAP_BY_ARRAY);
}
}

public static void pushListOfString(GeneratorAdapter methodVisitor, List<String> names) {
if (names != null) {
names = names.stream().filter(Objects::nonNull).toList();
}
if (names == null || names.isEmpty()) {
invokeInterfaceStatic(methodVisitor, LIST_TYPE, LIST_OF[0]);
return;
}
if (names.size() < LIST_OF.length) {
for (String name : names) {
methodVisitor.push(name);
}
invokeInterfaceStatic(methodVisitor, LIST_TYPE, LIST_OF[names.size()]);
} else {
int totalSize = names.size();
// start a new array
pushNewArray(methodVisitor, String.class, totalSize);
int i = 0;
for (String name : names) {
pushStoreStringInArray(methodVisitor, i++, totalSize, name);
}
invokeInterfaceStatic(methodVisitor, LIST_TYPE, LIST_BY_ARRAY);
}
}

private static void invokeInterfaceStatic(GeneratorAdapter methodVisitor, Type type, org.objectweb.asm.commons.Method method) {
methodVisitor.visitMethodInsn(INVOKESTATIC, type.getInternalName(), method.getName(), method.getDescriptor(), true);
}

}
Loading

0 comments on commit d65a608

Please sign in to comment.