Skip to content

Conversation

Copy link

Copilot AI commented Nov 26, 2025

PR google#234 to google/emboss failed CI due to Black formatting violations and outdated golden test files after introducing the two-pass subexpression caching optimization for conditional fields.

Changes

  • Python formatting: Applied Black 24.8.0 to run_one_golden_test.py and header_generator.py
  • Golden file regeneration: Updated all 33 golden header files in testdata/golden_cpp/ to reflect the new Ok() method generation pattern

The new code generation wraps field existence checks in scoped blocks with cached subexpressions:

// Before
if (!has_x().Known()) return false;
if (has_x().ValueOrDefault() && !x().Ok()) return false;

// After
{
  const auto emboss_reserved_local_field_present = ${existence_condition};
  if (!emboss_reserved_local_field_present.Known()) return false;
  if (emboss_reserved_local_field_present.ValueOrDefault() && !x().Ok()) return false;
}

Special handling applied for no_enum_traits.emb.h (requires --no-cc-enum-traits flag).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • releases.bazel.build
    • Triggering command: /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 build //compiler/back_end/cpp:emboss_codegen_cpp //compiler/front_end:emboss_front_end (dns block)
  • www.googleapis.com
    • Triggering command: /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 /usr/local/lib/node_modules/@bazel/bazelisk/bazelisk-linux_amd64 build //... (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

Fix my fork so that all targets in all BUILD files build correctly and all tests pass. Fix the code so that the presummit tests in pending pull request I submitted to google/emboss pass.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

AaronWebster and others added 6 commits November 25, 2025 12:55
Optimized the generated C++ code for structures with conditional fields
by identifying and caching repeated subexpressions (e.g., tag reads)
in the `Ok()`, `Equals()`, and `UncheckedEquals()` methods.

This approach uses a two-pass generation strategy:
1.  Count usage of existence condition subexpressions.
2.  Generate code where frequently used subexpressions are cached in
    local variables at the method scope, while unique expressions are
    inlined.

This reduces redundant computations (CPU optimization) without
introducing excessive stack usage for unique conditions (Memory
optimization).

Also updated test infrastructure to support passing compiler flags to
golden tests, fixing `no_enum_traits_golden_test`.

Added `testdata/many_conditionals.emb` and `compiler/back_end/cpp/testcode/many_conditionals_benchmark.cc`
to verify performance of conditional fields.

Performance Analysis (10,000 iterations x 100 tags on `testdata/many_conditionals.emb`):
- Baseline Runtime: ~1.83s
- Optimized Runtime: ~0.21s (Speedup: ~8.7x)

Binary Size Impact (many_conditionals_benchmark, -c opt):
- Baseline: 57848 bytes (text: 56112)
- Optimized: 30472 bytes (text: 28736) -> ~47% reduction
Copilot AI changed the title [WIP] Fix build targets and ensure all tests pass Fix C++ backend conditional optimization: formatting and golden file regeneration Nov 26, 2025
Copilot AI requested a review from AaronWebster November 26, 2025 00:24
Copilot finished work on behalf of AaronWebster November 26, 2025 00:24
@AaronWebster AaronWebster marked this pull request as ready for review November 26, 2025 00:28
@AaronWebster AaronWebster merged commit 990e035 into master Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants