<div dir="ltr"><div>Hi Maurizio, thanks for the comments!<br><br>I am still kind of on the fence about this one, as you say.<br><br>I collected some data from Google's codebase, and a majority of enum constants would be able to use condy:<br>* 71% of enum declarations have no explicit constructors that take parameters.</div><div>* 80% percent of expressions used as arguments to enum constructors are constants.<br>* The most common non-constant expressions used as arguments to enum constructors include static factory methods (e.g. `List.of(...)`, `Map.of(...)`, Optional.of(...)`). I also saw a number of static field accesses (e.g. `BigDecimal.ZERO`). None of those individual examples account for more than 1% of the total, and there was a long tail.</div><div><br>What are your thoughts on how often we'd want to be able to use this path, before it was worth supporting?<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 10, 2023 at 3:34 AM Maurizio Cimadamore <<a href="mailto:maurizio.cimadamore@oracle.com" target="_blank">maurizio.cimadamore@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Liam,<br>
the overall idea seem fine. I'm a bit on the fence (as I think you <br>
already are) when I try to assess whether adding the extra complexity is <br>
worth it here. On the one hand, the code we have today is not very <br>
compact, but the javac translation strategy is quite uniform, and can <br>
deal with both enum construction and custom enums (e.g. enums which <br>
define their own bodies). The approach proposed here, while it's more <br>
compact for the case where an enum has no parameters (or where the <br>
parameters are not constant), it has to contain some fallback logic for <br>
the more general case, so the resulting lowering code becomes a little <br>
bit more cryptic to understand, and maintain (going forward).<br>
<br>
Do you have some data re. how many enum construction can be optimized <br>
with this approach? Trying to think about code I've seen in the past, <br>
the assumption that enum construction parameters are constant doesn't <br>
strike me as too unrealistic, so I would expect that a lot of enums out <br>
there could be optimized by this path (in which case I'd be more willing <br>
to consider it).<br>
<br>
Thanks for the great work!<br>
Maurizio<br>
<br>
<br>
On 09/10/2023 22:45, Liam Miller-Cushon wrote:<br>
> Hello,<br>
><br>
> I'd like to discuss the idea of using constant dynamic in the <br>
> generated code for enums.<br>
><br>
> The possibility of using condy came up in the review thread for <br>
> JDK-8241798, which implemented a small change to the code generation <br>
> strategy for enums to raise the limit on the number of enum constants. <br>
> To recap, there are limits on the number of constants in an enum <br>
> imposed by the class file format. The constants are represented as <br>
> static final fields, which have to be initialized in the clinit, which <br>
> eventually runs into the 64k method size limit:<br>
><br>
> <a href="https://mail.openjdk.org/pipermail/compiler-dev/2020-March/014413.html" rel="noreferrer" target="_blank">https://mail.openjdk.org/pipermail/compiler-dev/2020-March/014413.html</a><br>
> <a href="https://bugs.openjdk.org/browse/JDK-8241798" rel="noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-8241798</a><br>
><br>
> I did some recent experimentation into using condy to initialize enum <br>
> constants and create the values array. There's a very rough draft <br>
> here: <a href="https://github.com/openjdk/jdk/pull/16108" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/pull/16108</a><br>
><br>
> With that change the clinit just contains a ldc/putstatus for each <br>
> constant, which is 6 bytes per constant, raising the limit on the <br>
> number of constants from ~4k today to about 10k.<br>
><br>
> I am interested in high level feedback about the idea. Does this seem <br>
> like a good use of condy, or a bad use of condy? Are there reasons <br>
> (performance, maintenance, other things I'm not considering) why using <br>
> condy in this way wouldn't be advisable?<br>
><br>
> One caveat is that it may not be very valuable to raise the limit <br>
> further. Four thousand is already a lot of enum constants. I suspect <br>
> for many domains where it's necessary to represent >4k values, it will <br>
> eventually need to represent >10k values, and that enums are not the <br>
> best way to model those domains. So I wouldn't argue for doing this if <br>
> there end up being a lot of trade-offs, but if the condy approach was <br>
> appealing in general perhaps raising the limit at the same time could <br>
> be win-win.<br>
><br>
> Working through the prototype raised some questions about what a <br>
> production quality implementation would look like:<br>
><br>
> Enums with user-defined constructors are currently only handled if the <br>
> arguments are static constants. What would the right solution be? <br>
> Perhaps a factory method could be synthetized for constants with <br>
> non-constant arguments, so condy could still be used to load them.<br>
><br>
> The class initializer for MethodHandle transitively loads enums, so <br>
> having enum class initializers use condy creates a cyclic dependency. <br>
> The prototype skips the new strategy for java.base. I could imagine <br>
> having a way to opt specific enums out of the new strategy, but having <br>
> to maintain multiple code generation strategies for enums does not <br>
> seem very appealing. Perhaps there's a way to refactor to break those <br>
> cycles, or that the lazy static final field proposal in JDK-8209964 <br>
> could help here, or something else.<br>
><br>
> I'm curious what you think,<br>
> Liam<br>
</blockquote></div>