Skip to content

BuiltinResult should only have two constructors #7507

@effectfully

Description

@effectfully

BuiltinResult has this long comment about the impossibility of ensuring that branch prediction predicts the right branch (i.e. BuiltinSuccess):

data BuiltinResult a
  = -- 'BuiltinSuccess' is the first constructor to make it a bit more likely for GHC to
    -- branch-predict it (which is something that we want, because most builtins return this
    -- constructor). It is however not guaranteed that GHC will predict it, because even though
    -- it's likely going to be a recursive case (it certainly is in the CEK machine) and thus the
    -- constructor has precedence over 'BuiltinFailure', it doesn't have precedence over
    -- 'BuiltinSuccessWithLogs', since that case is equally likely to be recursive.
    --
    -- Unfortunately, GHC doesn't offer any explicit control over branch-prediction (see this
    -- ticket: https://gitlab.haskell.org/ghc/ghc/-/issues/849), so relying on hope is the best we
    -- can do here.
    BuiltinSuccess a
  | BuiltinSuccessWithLogs (DList Text) a
  | BuiltinFailure (DList Text) BuiltinError

However maybe a way to improve the odds would be to make it

data LoggedBuiltinResult
  = LoggedBuiltinSuccess a
  | LoggedBuiltinFailure BuiltinError

data BuiltinResult a
  = BuiltinSuccess a
  | LoggedBuiltinResult (DList Text) a

and then sprinkle at the call site (i.e. the CEK machine) what SPJ describes in the referenced ticket:

The best approximating effect that I can use at the moment is to make
the unlikely realloc branch a local function in a where clause but mark
it with {-# NOINLINE #-} so that the code for the realloc case doesn't
pollute the tight loop for the normal fast case.

Or at least do the latter without refactoring BuiltinResult into two data types.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions