Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid returning / passing around a huge ParsedDeclaration type #16954

Merged
merged 6 commits into from May 20, 2017
Merged

Commits on May 19, 2017

  1. Avoid returning / passing around a huge ParsedDeclaration type

    This enum type used to contain the result of parsing
    one CSS source declaration (`name: value;`) and expanding shorthands.
    Enum types are as big as the biggest of their variant (plus discriminant),
    which was quite big because some shorthands
    expand to many longhand properties.
    This type was returned through many functions and methods,
    wrapped and rewrapped in `Result` with different error types.
    This presumably caused significant `memmove` traffic.
    
    Instead, we now allocate an `ArrayVec` on the stack
    and pass `&mut` references to it for various functions to push into it.
    This type is also very big, but we never move it.
    
    We still use an intermediate data structure because we sometimes decide
    after shorthand expansion that a declaration is invalid after all
    and that we’re gonna drop it.
    Only later do we push to a `PropertyDeclarationBlock`,
    with an entire `ArrayVec` or nothing.
    
    In future work we can try to avoid a large stack-allocated array,
    and instead writing directly to the heap allocation
    of the `Vec` inside `PropertyDeclarationBlock`.
    However this is tricky:
    we need to preserve this "all or nothing" aspect
    of parsing one source declaration,
    and at the same time we want to make it as little error-prone as possible
    for the various call sites.
    `PropertyDeclarationBlock` curently does property deduplication
    incrementally: as each `PropertyDeclaration` is pushed,
    we check if an existing declaration of the same property exists
    and if so overwrite it.
    To get rid of the stack allocated array we’d need to somehow
    deduplicate separately after pushing multiple `PropertyDeclaration`.
    SimonSapin committed May 19, 2017
  2. Work around CI bustage

    SimonSapin committed May 19, 2017
You can’t perform that action at this time.