RFR: JDK-8241798: Allow enums to have more constants
forax at univ-mlv.fr
forax at univ-mlv.fr
Mon Mar 30 17:15:33 UTC 2020
> De: "Brian Goetz" <brian.goetz at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "Liam Miller-Cushon" <cushon at google.com>, "compiler-dev"
> <compiler-dev at openjdk.java.net>
> Envoyé: Lundi 30 Mars 2020 15:45:06
> Objet: Re: RFR: JDK-8241798: Allow enums to have more constants
> Remi is of course correct that there are many creative solutions using
> indy/condy. I don’t want to stifle that creativity, but I also don’t want that
> creativity to stop Liam from making progress.
Thinking more about that, condy is interesting because it avoid the initialization of the array until the first time you use it,
something we should do in the long term but as part of an effort in the future to try to get ride of most of the existing static blocks,
the problem is that we are not yet there.
> Liam observed a problem, and came up with a narrowly targeted, low risk solution
> that doesn’t reopen questions about translation strategy and that is obviously
> better than what we have. Yes, we could discuss whether there’s a better
> big-picture strategy for dealing with the values() method, or for pushing the
> limit up higher. But let’s also be aware that it is too easy to get mired in
> blue-sky ideas and then Liam’s problem is not solved.
> So, +1 from me on this patch. It is simple and responsible.
About the patch, it's ok for me if the method $values() is not declared 'final' (because it is already static).
Rémi
---
For the record, here is how to use constant dynamic:
import static java.lang.constant.ConstantDescs.CD_Class;
import static java.lang.constant.ConstantDescs.CD_MethodHandles_Lookup;
import static java.lang.constant.ConstantDescs.CD_Object;
import static java.lang.constant.ConstantDescs.CD_String;
import static java.util.stream.IntStream.range;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
import static org.objectweb.asm.Opcodes.ACC_SUPER;
import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.ARETURN;
import static org.objectweb.asm.Opcodes.BIPUSH;
import static org.objectweb.asm.Opcodes.CHECKCAST;
import static org.objectweb.asm.Opcodes.DUP;
import static org.objectweb.asm.Opcodes.H_INVOKESTATIC;
import static org.objectweb.asm.Opcodes.ICONST_0;
import static org.objectweb.asm.Opcodes.ILOAD;
import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
import static org.objectweb.asm.Opcodes.NEW;
import static org.objectweb.asm.Opcodes.PUTSTATIC;
import static org.objectweb.asm.Opcodes.RETURN;
import static org.objectweb.asm.Opcodes.SIPUSH;
import static org.objectweb.asm.Opcodes.V11;
import java.lang.constant.MethodTypeDesc;
import java.lang.reflect.Array;
import java.nio.file.Files;
import java.nio.file.Path;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.ConstantDynamic;
import org.objectweb.asm.Handle;
import org.objectweb.asm.MethodVisitor;
public class GenerateMiniEnum {
private static final Handle BSM = new Handle(H_INVOKESTATIC,
RT.class.getName().replace('.', '/'),
"getEnumConstants",
MethodTypeDesc
.of(CD_Object, CD_MethodHandles_Lookup, CD_String, CD_Class, CD_Object.arrayType()).descriptorString(),
false);
private static void generateInt(MethodVisitor mv, int value) {
if (value >= -1 && value <= 5) {
mv.visitInsn(ICONST_0 + value);
return;
}
if (value >= Byte.MIN_VALUE && value <= Byte.MAX_VALUE) {
mv.visitIntInsn(BIPUSH, value);
return;
}
if (value >= Short.MIN_VALUE && value <= Short.MAX_VALUE) {
mv.visitIntInsn(SIPUSH, value);
return;
}
mv.visitLdcInsn(value);
}
private static byte[] generate(int enumConstantCount) {
var enumNames = range(0, enumConstantCount).mapToObj(i -> "V" + i).toArray();
var writer = new ClassWriter(0);
writer.visit(V11, ACC_PUBLIC | ACC_SUPER, "MyEnum", null, "java/lang/Enum", null);
for(var i = 0; i <enumConstantCount; i++) {
var fv = writer.visitField(ACC_PUBLIC | ACC_STATIC | ACC_FINAL, "V" + i, "LMyEnum;", null, null);
fv.visitEnd();
}
var init = writer.visitMethod(ACC_PRIVATE, "<init>", "(Ljava/lang/String;I)V", null, null);
init.visitCode();
init.visitVarInsn(ALOAD, 0);
init.visitVarInsn(ALOAD, 1);
init.visitVarInsn(ILOAD, 2);
init.visitMethodInsn(INVOKESPECIAL, "java/lang/Enum", "<init>", "(Ljava/lang/String;I)V", false);
init.visitInsn(RETURN);
init.visitMaxs(3, 3);
init.visitEnd();
var values = writer.visitMethod(ACC_PUBLIC | ACC_STATIC, "values", "()[LMyEnum;", null, null);
values.visitCode();
var condy = new ConstantDynamic("values", "[LMyEnum;", BSM, enumNames );
values.visitLdcInsn(condy);
values.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Object", "clone", "()Ljava/lang/Object;", false);
values.visitTypeInsn(CHECKCAST, "[LMyEnum;");
values.visitInsn(ARETURN);
values.visitMaxs(1, 0);
values.visitEnd();
var clinit = writer.visitMethod(ACC_STATIC, "<clinit>", "()V", null, null);
clinit.visitCode();
for(var i = 0; i < enumConstantCount; i++) {
clinit.visitTypeInsn(NEW, "MyEnum");
clinit.visitInsn(DUP);
clinit.visitLdcInsn("V" + i);
generateInt(clinit, i);
clinit.visitMethodInsn(INVOKESPECIAL, "MyEnum", "<init>", "(Ljava/lang/String;I)V", false);
clinit.visitFieldInsn(PUTSTATIC, "MyEnum", "V" + i, "LMyEnum;");
}
clinit.visitInsn(RETURN);
clinit.visitMaxs(4, 0);
clinit.visitEnd();
return writer.toByteArray();
}
private static Class<?> array(Class<?> type) {
return Array.newInstance(type, 0).getClass();
}
public static void main(String[] args) throws Throwable {
var path = Path.of("MyEnum.class");
var outputFolder = Path.of("minified");
Files.createDirectories(outputFolder);
var byteArray = generate(4100);
Files.write(outputFolder.resolve(path), byteArray);
}
}
>> On Mar 28, 2020, at 8:12 PM, Remi Forax < [ mailto:forax at univ-mlv.fr |
>> forax at univ-mlv.fr ] > wrote:
>> Hi Liam,
>> Having values() initialized is the static block is a common issue once you have
>> a code generator that spit enums.
>> I had a similar issue and ends up not using enums but plain old classes in the
>> generator because of that.
>> There is another solution,
>> you can use constantdynamic and pass the names of the constants (an array) as a
>> bootstrap argument,
>> you can go up to 65 635 (the limit in term of number of fields), in fact far
>> less because the constant pool will be full before that point.
>> public static MyEnum[] values() {
>> ldc "values" [MyEnum [BSM=java/lang/runtime/ConstantEnumMetaFactory]
>> ["constant1", "constant2", ...]
>> invokevirtual clone()
>> areturn
>> }
>> The major advantage of using constant dynamic is that if nobody calls the method
>> values(), the array is never initialized.
>> And once the VM will suport fields declared as "lazy static final", you can get
>> ride of the static bock because the initialization of the constant fields will
>> be done lazily too.
>> Rémi
>>> De: "Liam Miller-Cushon" < [ mailto:cushon at google.com | cushon at google.com ] >
>>> À: "compiler-dev" < [ mailto:compiler-dev at openjdk.java.net |
>>> compiler-dev at openjdk.java.net ] >
>>> Envoyé: Dimanche 29 Mars 2020 00:37:24
>>> Objet: RFR: JDK-8241798: Allow enums to have more constants
>>> Please consider this change to allow enums to have ~4100 constants (up from the
>>> current limit of ~2740), by moving the creation of the values array out of the
>>> clinit to a helper method.
>>> It's fair to ask whether this is worth doing. Increasing the limit by this
>>> amount doesn't really change the use-cases enums are suitable for: they still
>>> can't represent arbitrarily large collections of constants, and if you have an
>>> enum with >2740 constants you're probably going to want >4100 eventually. On
>>> the other hand, I don't see any obvious drawbacks to the change (in terms of
>>> complexity in the compiler, performance impact, or complexity of generated
>>> code). And other options for increasing the limit would require significantly
>>> more difficult and longer-term changes (e.g. tackling the method and constant
>>> pool size limits).
>>> Another question is whether this is the best code generation strategy. Currently
>>> javac saves the values array to a field and returns copies of it using clone().
>>> The approach ecj uses skips the field, and just re-creates the array every time
>>> values() is called. For now I'm keeping the values field to minimize the
>>> performance impact of this change, but the ecj approach would avoid that field
>>> and the helper method.
>>> bug: [ https://bugs.openjdk.java.net/browse/JDK-8241798 |
>>> https://bugs.openjdk.java.net/browse/JDK-8241798 ]
>>> webrev: [ http://cr.openjdk.java.net/~cushon/8241798/webrev.00/ |
>>> http://cr.openjdk.java.net/~cushon/8241798/webrev.00/ ]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200330/cf2c6540/attachment-0001.htm>
More information about the compiler-dev
mailing list