Built Class with too many fields causes code generation to fail due to too much nesting in generated hashCode about built_value.dart HOT 6 CLOSED

I have now asked, will update if there's anything useful :) thanks.

from built_value.dart.

Thanks!

I'm not sure if the various compilers/runtimes (VM jit, aot, ddc, dart2js) will produce equivalent code for the proposed implementation; hashing is performance critical so any change could matter.

I can see three ways forward:

Benchmark on the various compilers/runyimes to check;

Or, ask the owners of the various compilers/runtimes to check :) I'm on the Dart team so I could do that, I guess it's not super high priority for everyone but they will probably find the question entertaining so we may get a clear answer fairly quickly :)

Or, we could change the generated code just for e.g. 18+ fields. I'm not worried about any small performance difference for such an unusual case.

What do you think? Thanks :)

from built_value.dart.

When preparing #1199, I actually checked for differences in performance on my machine (ARM64) in AOT time (using dart compile exe).

I ran the hashcode benchmark in the benchmark/ package and added a second benchmark with a built_value class with 50 fields. These were my results, I ran the benchmarks 3 times:

current, nested hash:

hashCode many fields: 215442951/s
hashCode many fields: 214758182/s
hashCode many fields: 216637782/s

proposed change, flat:

hashCode many fields: 215169446/s
hashCode many fields: 216915034/s
hashCode many fields: 216642475/s

So at least for ARM64/AOT, there seem to be no significant difference.
I did not check any other targets though, and did not inspect the generated code, so it's possible that there regressions on other platforms and it's possible that there are differences that are too small to be detected in this benchmark.

Obviously, I would get excited if we got insights from the Dart team on that matter :)

But I would also not mind modifying #1199 to only change the generated code for classes with more than X fields.

from built_value.dart.

How urgent is it from your side? I could land and release a new version with the generation change for large classes this week, probably.

I think I will anyway ask the Dart team thr more general question of what would be the best generated hashing code.

Thanks.

from built_value.dart.

Not very urgent for me.
It's affecting one user of one of the packages that I maintain (ferry https://pub.dev/packages/ferry).

Ferry generates built_value classes from a graphql schema/queries.

They found a workaround though, so I guess they don't need an immediate fix either.

If we get a response from the Dart team within a few weeks that's totally fine by me.

from built_value.dart.

fixed by #1199

from built_value.dart.