-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do an optimization pass on platform-specific output #204
Comments
Here's a quick one. The protoOf(Simple_0).toString = function () {
return 'Simple(int=' + this.int_1 + ', requiredString=' + this.requiredString_1 + ', optionalString=' + this.optionalString_1 + ')';
}; Perfectly reasonable for JS. On the JVM and Android, however, strings are used both for string literals but also for the names of types, methods, and fields. This means that the backing fields for the
Despite being counter-intuitive, it's actually more efficient to generate code which looks like this return "Simple" + '(' + "int" + '=' + int + ", " + "requiredString" + '=' + ... This will allow sharing the string bytes with the type and field names, and it means the Generating this is tricky, because you have to avoid any steps that join constants together. I have no idea if we can do it in IR, but I would suspect it's possible. It may also only be beneficial to do this for JVM targets. I suspect a good JS minifier will combine the constant strings back together (which is good, size is more important), but I have no idea what the correct thing to do for native targets is. I previously did this optimization on View Binding's generated code: https://jakewharton.com/the-economics-of-generated-code/#string-duplication. It'd be another selling point of Poko over data classes. Separately, seeing if |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Equality checking has suboptimal control flow. The Java bytecode:
I have manually separated these into what I hope is somewhat clear operations. It's successive sets of loading two corresponding values, doing a comparison, and then maybe returning false. Finally, we return true. The control flow graph looks somewhat like this:
I see two things wrong with this:
Now, I'm not sure which is the happy path: equals or non-equals, but it doesn't really matter. We should pick one and optimize for it. I am going to pick the equals path as being the happy path under the assumption that we want to optimize for hash-based collection usage. These will use the hash code to find the bucket and then do an equals to ensure it found a match or a collision requiring another check. An ideal hash map has no hash collisions, so this equals check should always succeed. I also think equals is worth optimizing for simply because it will produce more compact bytecode, which should look something like this:
Now the equals case flows linearly from top to |
Content-based array IR uses inefficient property access causing a performance problem. Consider just protoOf(GenericArrayHolder).hashCode = function () {
var tmp;
if (this.generic_1 == null) {
tmp = 0;
} else {
var tmp_0;
var tmp_1 = this.generic_1;
if (!(tmp_1 == null) ? isArray(tmp_1) : false) {
tmp_0 = contentDeepHashCode(this.generic_1);
} else {
var tmp_2 = this.generic_1;
if (!(tmp_2 == null) ? isBooleanArray(tmp_2) : false) {
tmp_0 = contentHashCode_6(this.generic_1);
} else {
var tmp_3 = this.generic_1;
if (!(tmp_3 == null) ? isCharArray(tmp_3) : false) {
tmp_0 = contentHashCode_5(this.generic_1);
} else {
var tmp_4 = this.generic_1;
if (!(tmp_4 == null) ? isByteArray(tmp_4) : false) {
tmp_0 = contentHashCode_4(this.generic_1);
} else {
var tmp_5 = this.generic_1;
if (!(tmp_5 == null) ? isShortArray(tmp_5) : false) {
tmp_0 = contentHashCode_3(this.generic_1);
} else {
var tmp_6 = this.generic_1;
if (!(tmp_6 == null) ? isIntArray(tmp_6) : false) {
tmp_0 = contentHashCode_2(this.generic_1);
} else {
var tmp_7 = this.generic_1;
if (!(tmp_7 == null) ? isFloatArray(tmp_7) : false) {
tmp_0 = contentHashCode_1(this.generic_1);
} else {
var tmp_8 = this.generic_1;
if (!(tmp_8 == null) ? isLongArray(tmp_8) : false) {
tmp_0 = contentHashCode_0(this.generic_1);
} else {
var tmp_9 = this.generic_1;
if (!(tmp_9 == null) ? isDoubleArray(tmp_9) : false) {
tmp_0 = contentHashCode(this.generic_1);
} else {
tmp_0 = this.generic_1 == null ? 0 : hashCode(this.generic_1);
}
}
}
}
}
}
}
}
}
tmp = tmp_0;
}
return tmp;
}; Every reference to This is also evident in the Java bytecode:
There are way too many I assume the same is true for native... And this also applies to |
From content-based array
We are calling a JDK function (good!) but Kotlin is mistrusting its return value and adding a non-null check (bad!). Worse, this non-null check comes with a string representation of the expression which is just wasted bytes. We should somehow convince the IR to trust this method call's platform return type as a non-null return type. This would eliminate the method call and eliminate the useless string. It should be possible given the presence of functions like |
We want to optimize for a sweet spot between performance and code size.
This is mostly a placeholder issue until #142 is sufficiently resolved that this becomes a priority and the research is done to see where any problems may lie.toString()
string de-duplication with types/fieldsinstanceof
Int.hashCode()
callhashCode
calculation #216UInt.hashCode()
call. Same as above, should betoInt()
call which is no-op.hashCode
forUInt
#230equals()
checkNonNull
instrinsicThe text was updated successfully, but these errors were encountered: