Share UKM name string table between ukm_builders and ukm_decode [chromium/src : main]

[Andrew Grieve (Gerrit)]

Attention needed from Helmut Januschka

Andrew Grieve voted and added 4 comments

Votes added by Andrew Grieve

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Andrew Grieve . resolved

lgtm, but kinda puzzled over why this has no size impact.

Commit Message

Line 11, Patchset 2 (Latest):constants, removing ~84 KB of duplicated string data.

Andrew Grieve . unresolved

hmmm... android-binary-size shows this has no impact on size at all... My only guess is that all of these strings exist somewhere else in the binary as well, so this is just changing the constants from pointers into the global string pool to pointers into the manual string pool.

File tools/metrics/ukm/codegen.py

Line 48, Patchset 2 (Latest): offset += len(name) + 1

Andrew Grieve . unresolved

Nit add comment that the table itself does have duplicates, so still need to step over them.

File tools/metrics/ukm/names_template.py

Line 21, Patchset 2 (Latest):namespace ukm {{

Andrew Grieve . unresolved

nit: `namespace ukm::builders::internal {{`

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ie420a7bc98e2dfc566621fcbb99cd8138e16db43

Gerrit-Change-Number: 8215212

Gerrit-PatchSet: 2

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Andrew Grieve <agr...@chromium.org>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Comment-Date: Fri, 07 Aug 2026 03:25:55 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Helmut Januschka (Gerrit)]

Attention needed from Andrew Grieve

Helmut Januschka added 3 comments

Commit Message

Line 11, Patchset 2:constants, removing ~84 KB of duplicated string data.

Andrew Grieve . unresolved

hmmm... android-binary-size shows this has no impact on size at all... My only guess is that all of these strings exist somewhere else in the binary as well, so this is just changing the constants from pointers into the global string pool to pointers into the manual string pool.

Helmut Januschka

checked this locally and the mechanism is slightly different: the strings aren't shared with a pool elsewhere, they were never in the shipped binary at all. The kEntryName / k*Name arrays are referenced almost exclusively from test code, so --gc-sections already strips them from chrome. I reproduced it with a release-style link (--gc-sections --icf=all): rodata is byte-identical with and without this patch when only the decode map is referenced, which matches the bot's zero delta.

The dedup is still real for binaries that do reference the name constants

File tools/metrics/ukm/codegen.py

Line 48, Patchset 2: offset += len(name) + 1

Andrew Grieve . resolved

Nit add comment that the table itself does have duplicates, so still need to step over them.

Helmut Januschka

Done

File tools/metrics/ukm/names_template.py

Line 21, Patchset 2:namespace ukm {{

Andrew Grieve . resolved

nit: `namespace ukm::builders::internal {{`

Helmut Januschka

Done

Open in Gerrit

Related details

Attention is currently required from:

• Andrew Grieve

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ie420a7bc98e2dfc566621fcbb99cd8138e16db43

Gerrit-Change-Number: 8215212

Gerrit-PatchSet: 4

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Andrew Grieve <agr...@chromium.org>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Andrew Grieve <agr...@chromium.org>

Gerrit-Comment-Date: Fri, 07 Aug 2026 12:02:45 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Andrew Grieve <agr...@chromium.org>

[Andrew Grieve (Gerrit)]

Attention needed from Helmut Januschka and Ramon Cano Aparicio

Andrew Grieve voted and added 1 comment

Votes added by Andrew Grieve

Code-Review

+1

1 comment

Commit Message

Line 11, Patchset 2:constants, removing ~84 KB of duplicated string data.

Andrew Grieve . resolved

hmmm... android-binary-size shows this has no impact on size at all... My only guess is that all of these strings exist somewhere else in the binary as well, so this is just changing the constants from pointers into the global string pool to pointers into the manual string pool.

Helmut Januschka

checked this locally and the mechanism is slightly different: the strings aren't shared with a pool elsewhere, they were never in the shipped binary at all. The kEntryName / k*Name arrays are referenced almost exclusively from test code, so --gc-sections already strips them from chrome. I reproduced it with a release-style link (--gc-sections --icf=all): rodata is byte-identical with and without this patch when only the decode map is referenced, which matches the bot's zero delta.

The dedup is still real for binaries that do reference the name constants

Andrew Grieve

Ah, mystery solved then.

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka
• Ramon Cano Aparicio

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ie420a7bc98e2dfc566621fcbb99cd8138e16db43

Gerrit-Change-Number: 8215212

Gerrit-PatchSet: 4

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Andrew Grieve <agr...@chromium.org>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Ramon Cano Aparicio <rcanoa...@google.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Attention: Ramon Cano Aparicio <rcanoa...@google.com>

Gerrit-Comment-Date: Fri, 07 Aug 2026 13:42:28 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

Comment-In-Reply-To: Helmut Januschka <hel...@januschka.com>

Comment-In-Reply-To: Andrew Grieve <agr...@chromium.org>

[Helmut Januschka (Gerrit)]

Attention needed from Andrew Grieve and Ramon Cano Aparicio

Helmut Januschka added 1 comment

Patchset-level comments

File-level comment, Patchset 2:

Andrew Grieve . unresolved

lgtm, but kinda puzzled over why this has no size impact.

Helmut Januschka

after landing of the first CL, changed HEAD to main, its now at +4B, wdyt, i am thinking about abandoning it!

Open in Gerrit

Related details

Attention is currently required from:

• Andrew Grieve
• Ramon Cano Aparicio

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ie420a7bc98e2dfc566621fcbb99cd8138e16db43

Gerrit-Change-Number: 8215212

Gerrit-PatchSet: 5

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Andrew Grieve <agr...@chromium.org>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Ramon Cano Aparicio <rcanoa...@google.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Andrew Grieve <agr...@chromium.org>

Gerrit-Attention: Ramon Cano Aparicio <rcanoa...@google.com>

Gerrit-Comment-Date: Fri, 07 Aug 2026 21:28:39 +0000

[Andrew Grieve (Gerrit)]

Attention needed from Helmut Januschka and Ramon Cano Aparicio

Andrew Grieve voted and added 1 comment

Votes added by Andrew Grieve

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 2:

Andrew Grieve . resolved

lgtm, but kinda puzzled over why this has no size impact.

Helmut Januschka

after landing of the first CL, changed HEAD to main, its now at +4B, wdyt, i am thinking about abandoning it!

Andrew Grieve

yeah, without any wins, probably not worth the added complexity.

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka
• Ramon Cano Aparicio

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Ie420a7bc98e2dfc566621fcbb99cd8138e16db43

Gerrit-Change-Number: 8215212

Gerrit-PatchSet: 5

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Andrew Grieve <agr...@chromium.org>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Ramon Cano Aparicio <rcanoa...@google.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Attention: Ramon Cano Aparicio <rcanoa...@google.com>

Gerrit-Comment-Date: Sat, 08 Aug 2026 00:44:28 +0000

[Helmut Januschka (Gerrit)]

Helmut Januschka abandoned this change.

View Change

Abandoned

Helmut Januschka abandoned this change

Related details

Attention set is empty

Submit Requirements:

• Code-Coverage

• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: abandon