Use a flag array and indices in runtime feature lookup tables [chromium/src : main]

0 views

Skip to first unread message

Helmut Januschka (Gerrit)

unread,

7:09 PM (1 hour ago) 7:09 PM

to Andrew Grieve, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org

Attention needed from Andrew Grieve

Helmut Januschka added 4 comments

File third_party/blink/renderer/build/scripts/templates/runtime_enabled_features.cc.tmpl

Line 80, Patchset 3: {% for feature in features|sort(attribute='name') if feature.is_protected_feature %}

Andrew Grieve . resolved

nit: should add a comment that there are only a few protected features.

Helmut Januschka

Done.

Line 91, Patchset 3: bool* flag;

Andrew Grieve . resolved

There's likely also a decent savings to change all the static fields to a single `bool[]` array, and use indices.

Helmut Januschka

Done. Replaced the per-feature static bools with a single feature_states_[] array indexed by a generated enum (uint16_t), and the lookup/mapping tables now store indices instead of bool* pointers

Line 140, Patchset 3: // keeps this table free of generated setter instantiations. Protected

Andrew Grieve . resolved

nit: comment should say why this is desirable (so that the functions need not exist after inlining)

Helmut Januschka

Done

Line 146, Patchset 3: bool if_overridden;

Andrew Grieve . resolved

nit: I think this is causing 3 (or 7) bytes of padding in the struct. Could move it to a separate bool[] to save a bunch of padding.

Helmut Januschka

Done. Split the mapping into parallel arrays (base::Feature*[] / uint16_t[] / bool[]), so entries cost 11 bytes instead of 24 (or 16 after the index change).

Open in Gerrit

Related details

Attention is currently required from:

Andrew Grieve

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

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages