Skip to content

Commit

Permalink
Emit argument array for contiguous numbered arguments
Browse files Browse the repository at this point in the history
For strings which match, this reduces the generated bytecode impact.
Each argument 'put' operation in the map required:

    36: aload         6
    38: ldc           #99                 // String 0
    40: aload_1
    41: invokevirtual #30                 // Method android/util/ArrayMap.put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
    44: pop

plus the associated string pool constant's size.

In the array, storage requires fewer bytecodes and bytecodes with fewer arguments reducing their size:

    32: aload         7
    34: iconst_0
    35: aload_1
    36: aastore

Creating the map used to cost 10 bytes and require invoking the ArrayMap constructor:

    26: new           #23                 // class android/util/ArrayMap
    29: dup
    30: iconst_5
    31: invokespecial #26                 // Method android/util/ArrayMap."<init>":(I)V
    34: astore        6

But the array requires only 5 bytes and zero method calls:

    27: anewarray     #4                  // class java/lang/Object
    30: astore        7

Finally, previously, the `Map` property of the `FormattedResource` class required an explicit cast inside the generated code:

     89: new           #32                // class app/cash/gingham/FormattedResource
     92: dup
     93: ldc           #108               // int 2131689631
     95: aload         6
     97: checkcast     #35                // class java/util/Map
    100: invokespecial #38                // Method app/cash/gingham/FormattedResource."<init>":(ILjava/util/Map;)V
    103: areturn

But with the property changing to `Any` (i.e., `Object`) the cast is no longer required for either the array-based or map-based generated code:

    69: new           #32                 // class app/cash/gingham/FormattedResource
    72: dup
    73: ldc           #96                 // int 2131689631
    75: aload         6
    77: invokespecial #36                 // Method app/cash/gingham/FormattedResource."<init>":(ILjava/lang/Object;)V
    80: areturn
  • Loading branch information
JakeWharton committed Jan 17, 2023
1 parent 5b03e8a commit 8f06759
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 12 deletions.
18 changes: 15 additions & 3 deletions plugin/src/main/java/app/cash/gingham/plugin/ResourceWriter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,22 @@ private fun TokenizedResource.toFunSpec(packageStringsType: TypeName): FunSpec {
.apply { if (description != null) addKdoc(description) }
.apply { tokens.forEach { addParameter(it.toParameterSpec()) } }
.returns(Types.FormattedResource)
.addStatement("val arguments = %T(%L)", Types.ArrayMap.parameterizedBy(STRING, ANY), tokens.size)
.apply {
for (token in tokens) {
addStatement("arguments.put(%S, %L)", token.argumentName, token.toParameterCodeBlock())
if (hasContiguousNumberedTokens) {
addCode(
buildCodeBlock {
add("val arguments = arrayOf(⇥\n")
for (token in tokens) {
addStatement("%L,", token.toParameterCodeBlock())
}
add("⇤)\n")
}
)
} else {
addStatement("val arguments = %T(%L)", Types.ArrayMap.parameterizedBy(STRING, ANY), tokens.size)
for (token in tokens) {
addStatement("arguments.put(%S, %L)", token.argumentName, token.toParameterCodeBlock())
}
}
}
.addCode(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ internal data class TokenizedResource(
val description: String?,
val tokens: List<Token>,
) {
/* True when the tokens represent a contiguous set of integers counting from 0. */
val hasContiguousNumberedTokens: Boolean =
tokens.indices.toList() == tokens.map { (it as? Token.NumberedToken)?.number }

sealed interface Token {
val type: KClass<*>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ class ResourceWriterTest {
// Do not edit this file directly. Instead, edit the string resources in the source file.
package com.gingham.test
import android.util.ArrayMap
import app.cash.gingham.FormattedResource
import com.gingham.test.R
import java.time.Instant
Expand All @@ -104,12 +103,13 @@ class ResourceWriterTest {
arg3: Number,
arg4: String,
): FormattedResource {
val arguments = ArrayMap<String, Any>(5)
arguments.put("0", arg0)
arguments.put("1", Date.from(arg1))
arguments.put("2", arg2)
arguments.put("3", arg3)
arguments.put("4", arg4)
val arguments = arrayOf(
arg0,
Date.from(arg1),
arg2,
arg3,
arg4,
)
return FormattedResource(
id = R.string.test_numbered,
arguments = arguments
Expand Down
8 changes: 7 additions & 1 deletion runtime/src/main/java/app/cash/gingham/FormattedResource.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright Square, Inc.
package app.cash.gingham

import android.icu.text.MessageFormat
import androidx.annotation.StringRes

/**
Expand All @@ -23,5 +24,10 @@ import androidx.annotation.StringRes
* - The R.string.detective_has_suspects resource ID
* - An integer value for the suspects argument
* - A string value for the detective argument
*
* @property arguments Arguments passed directly to [MessageFormat.format].
*/
data class FormattedResource(@StringRes val id: Int, val arguments: Map<String, Any>)
data class FormattedResource constructor(
@StringRes val id: Int,
val arguments: Any,
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ fun Context.getString(formattedResource: FormattedResource): String =
* Resolves and returns the final formatted version of the given formatted string.
*/
fun Resources.getString(formattedResource: FormattedResource): String =
MessageFormat.format(getString(formattedResource.id), formattedResource.arguments)
MessageFormat(getString(formattedResource.id)).format(formattedResource.arguments)

0 comments on commit 8f06759

Please sign in to comment.