Skip to content

Commit

Permalink
Remove unnecessary VLOG from Transition
Browse files Browse the repository at this point in the history
Summary:
The `VLOG(8)` is not enabled anywhere (it looks like this was mainly for
debugging purposes during development) and this adds a non-trivial amount of
generated code. Each instantiation is distinct since the logging string is
dependent on the NTTP. The VLOG macro itself generates a decent amount of code
to check whether or not the logging level is enabled.

Additionally, downgrade the `CHECK` to a `DCHECK` -- we don't ever expect this
to be hit; the state machine implementations always use an unqualified access
to `Transition` (i.e. nobody ever explicitly writes `EventHandlerBase<SM, SomeOtherState, SomeOtherEvent>::Transition`, which means that the correct state is guaranteed. This also eliminates the
runtime check overhead.

Prior to this diff:
```
0000000000034ac4 <_ZN4fizz2sm16EventHandlerBaseINS_6client11ClientTypesELNS2_9StateEnumE1ELNS_5EventE3EJLS4_1EEE10TransitionILS4_1EEEvRNS2_5StateE>:
   34ac4:       d10103ff        sub     sp, sp, #0x40
   34ac8:       a90257fe        stp     x30, x21, [sp, #32]
   34acc:       a9034ff4        stp     x20, x19, [sp, #48]
   34ad0:       d53bd054        mrs     x20, tpidr_el0
   34ad4:       52800035        mov     w21, #0x1                       // #1
   34ad8:       f0fffea2        adrp    x2, b000 <note_end+0xad30>
   34adc:       911ea042        add     x2, x2, #0x7a8
   34ae0:       f9401688        ldr     x8, [x20, #40]
   34ae4:       910003e1        mov     x1, sp
   34ae8:       aa0003f3        mov     x19, x0
   34aec:       f9000fe8        str     x8, [sp, #24]
   34af0:       b90003f5        str     w21, [sp]
   34af4:       94003d92        bl      4413c <_ZN6google12Check_EQImplIN4fizz6client9StateEnumES3_EEPNSt6__ndk112basic_stringIcNS4_11char_traitsIcEENS4_9allocatorIcEEEERKT_RKT0_PKc>
   34af8:       f9000be0        str     x0, [sp, #16]
   34afc:       b5000140        cbnz    x0, 34b24 <_ZN4fizz2sm16EventHandlerBaseINS_6client11ClientTypesELNS2_9StateEnumE1ELNS_5EventE3EJLS4_1EEE10TransitionILS4_1EEEvRNS2_5StateE+0x60>
   34b00:       b9000275        str     w21, [x19]
   34b04:       f9401688        ldr     x8, [x20, #40]
   34b08:       f9400fe9        ldr     x9, [sp, #24]
   34b0c:       eb09011f        cmp     x8, x9
   34b10:       540001c1        b.ne    34b48 <_ZN4fizz2sm16EventHandlerBaseINS_6client11ClientTypesELNS2_9StateEnumE1ELNS_5EventE3EJLS4_1EEE10TransitionILS4_1EEEvRNS2_5StateE+0x84>  // b.any
   34b14:       a9434ff4        ldp     x20, x19, [sp, #48]
   34b18:       a94257fe        ldp     x30, x21, [sp, #32]
   34b1c:       910103ff        add     sp, sp, #0x40
   34b20:       d65f03c0        ret
   34b24:       f0fffea1        adrp    x1, b000 <note_end+0xad30>
   34b28:       9114e821        add     x1, x1, #0x53a
   34b2c:       910003e0        mov     x0, sp
   34b30:       910043e3        add     x3, sp, #0x10
   34b34:       52800502        mov     w2, #0x28                       // #40
   34b38:       940084bb        bl      55e24 <_ZN6google15LogMessageFatalC1EPKciRKNS_13CheckOpStringE>
   34b3c:       910003e0        mov     x0, sp
   34b40:       940080a1        bl      54dc4 <_ZN6google10LogMessage6streamEv>
   34b44:       14000002        b       34b4c <_ZN4fizz2sm16EventHandlerBaseINS_6client11ClientTypesELNS2_9StateEnumE1ELNS_5EventE3EJLS4_1EEE10TransitionILS4_1EEEvRNS2_5StateE+0x88>
   34b48:       94008c82        bl      57d50 <__stack_chk_fail@plt>
   34b4c:       910003e0        mov     x0, sp
   34b50:       940084b6        bl      55e28 <_ZN6google15LogMessageFatalD1Ev>
```

After this diff:

```
00000000000347e8 <_ZN4fizz2sm16EventHandlerBaseINS_6client11ClientTypesELNS2_9StateEnumE1ELNS_5EventE3EJLS4_1EEE10TransitionILS4_1EEEvRNS2_5StateE>:
   347e8:       52800028        mov     w8, #0x1                        // #1
   347ec:       b9000008        str     w8, [x0]
   347f0:       d65f03c0        ret
```

Reviewed By: abakiaydin

Differential Revision: D58162021

fbshipit-source-id: e223ac9902b3c6817ee4e5962292b20bd95c40f7
  • Loading branch information
Mingtao Yang authored and facebook-github-bot committed Jun 6, 2024
1 parent a036cce commit 870db84
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions fizz/protocol/StateMachine-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class EventHandlerBase {
static void Transition(typename SM::State& stateStruct) {
static_assert(
Or<StateSame<SM, to, AllowedStates>...>::value, "Transition invalid");
CHECK_EQ(stateStruct.state(), state);
VLOG(8) << "Transition from " << toString(state) << " to " << toString(to);
DCHECK_EQ(stateStruct.state(), state);
stateStruct.state() = to;
}
};
Expand Down

0 comments on commit 870db84

Please sign in to comment.