semantic: make cfg construction optional

[Boshen]

It's acting like a sleep(5000) for our transformer :-)

A preliminary memory profile revealed that we are using much more memory than swc

./files/cal.com.tsx
oxc 41.7 mb
swc 31.1 mb

./files/typescript.js
oxc 224.4 mb
swc 160.1 mb

[overlookmotel]

We could do this with either:

• a feature toggle (#[cfg(feature="control-flow-graph")])
• a generic param (Semantic<SemanticType>)

Personally, I prefer the generic. Either way, we'd need some way to determine whether a Semantic instance includes CFG or not.

On timing: In my view, it would be better to make this change after rzvxa has completed his overhaul of CFG, to avoid complicating further what's already a very complicated set of changes. So in my opinion it may not be realistic to include this in Transformer Milestone 1. There is a lot of potential for optimizing semantic in various ways, but I believe Milestone 1 is about getting the transformer working (including correct source maps and scopes, and thorough testing), and improving the performance is "phase 2" work.

[rzvxa]

I think both approaches are solid, It would really come down to whether we want to opt out of generating CFG code in build time entirely or we want more of a runtime optional scenario. We can also do such a thing with constant generics to optimize at compile time but still keep the generic syntax for those downstream projects that wish to have both options available without rebuilding.

Since I've been working on the control flow for the past few weeks - and counting - It wouldn't be so out of place if you guys let me know about your definitive preference and I get it done as part of my PR stack.

---

On another note; We might want to add memory footprint as part of our benchmarks so we don't increase it by mistake or underestimating the degree of impact a certain change would do to the overall memory usage. My guess is that we would mostly take performance over low memory usage where we have to choose between one or the other, But at least we can do it knowingly.

[Boshen]

Time wise please work on your own pace, it's not too urgent.

For now, I want a runtime control so that we can eventually tear things part and make all the builders inside the Semantic composable.

I'll eventually move the memory benchmark over to this repo, it's unstable right now because I don't really understand what I was doing :-)

[Boshen]

@rzvxa Rolldown is reaching unrechable!() s while bundling react and react-dom.

I think we need to put this issue on our radar 😅

[rzvxa]

I'll start working on this one right away

[Boshen]

are we going to brute force if enable_cfg checks everywhere 👀

[rzvxa]

For making it optional? If that's what you mean I was going with something like this(experimented with it for a while last night):

pub struct ComposableSemanticBuilder<
    'a,
    CFG: Cfg<C> = ControlFlowGraph,
    C: CFGBuilder<'a> = ControlFlowGraphBuilder<'a>,
> {
    pub source_text: &'a str,

    pub source_type: SourceType,

    trivias: Trivias,

    /// Semantic early errors such as redeclaration errors.
    errors: RefCell<Vec<OxcDiagnostic>>,

    // states
    pub current_node_id: AstNodeId,
    pub current_node_flags: NodeFlags,
    pub current_symbol_flags: SymbolFlags,
    pub current_scope_id: ScopeId,
    /// Stores current `AstKind::Function` and `AstKind::ArrowFunctionExpression` during AST visit
    pub function_stack: Vec<AstNodeId>,
    // To make a namespace/module value like
    // we need the to know the modules we are inside
    // and when we reach a value declaration we set it
    // to value like
    pub namespace_stack: Vec<SymbolId>,
    current_reference_flag: ReferenceFlag,

    // builders
    pub nodes: AstNodes<'a>,
    pub scope: ScopeTree,
    pub symbols: SymbolTable,

    pub(crate) module_record: Arc<ModuleRecord>,

    pub label_builder: LabelBuilder<'a>,

    jsdoc: JSDocBuilder<'a>,

    check_syntax_error: bool,

    pub cfg: C,
    _marker: PhantomData<CFG>,

    pub class_table_builder: ClassTableBuilder,

    ast_nodes_records: Vec<Vec<AstNodeId>>,
}

pub type SemanticBuilder<'a> =
    ComposableSemanticBuilder<'a, ControlFlowGraph, ControlFlowGraphBuilder<'a>>;

pub type QuickSemanticBuilder<'a> =
    ComposableSemanticBuilder<'a, NoControlFlowGraph, NoControlFlowGraphBuilder>;

What do you think about it? This way we won't need any checks we just implement a noop cfg and cfg builder.

[Boshen]

Can we get a brute force version, and then a nice version?

I'd like to unblock Rolldown from crashing 😅

[Boshen]

The API feels weird 🤔

pub struct ComposableSemanticBuilder<
    'a,
    CFG: Cfg<C> = ControlFlowGraph,
    C: CFGBuilder<'a> = ControlFlowGraphBuilder<'a>,
>

Are we able to expose some traits from the cfg builder so that the API becomes something like

trait CFGBuilder {
}

struct OxcCFGBuilder {
}

impl CFGBuilder for OxcCFGBuidler {
}

semantic_builder.with_cfg(OxcCFGBuilder)

[rzvxa]

No, the semantic builder needs to know its generic parts before being constructed. I was thinking that in simple cases consumers would just use the SemanticBuilder alias with no change to their current code.

In places where they want to have semantics without CFG they would use QuickSemanticBuilder in place of a SemanticBuilder.
The ComposableSemanticBuilder is mostly an internal type but more advanced users can use it to generate their own control flow and control flow builder.

A CFG trait is just a super trait of From<CFGBuilder> so they can generate any kind of graph from the vanilla builder(for example going for a more lean graph instead of our detailed one) or implement their own custom builder. I mostly chose this approach for experimentation to eliminate runtime checks or any possible dynamic dispatching that may occur with a dyn version. It would result in a larger binary since we have 2 versions of ComposableSemanticBuilder but you have to pay the price of performance somewhere I guess.

One advantage of this is that users can have NoControlFlowBuilder and yet they would still receive a control flow graph for example it can be used for testing/mocking purposes.

We can have different levels of control flow detail for different parts of our project, For example, less detailed (or none) for transformers yet have a really fine version for the minifier.
Most of these upsides are also available with the dynamic version but we have to pay for indirection and dispatch overhead.

I haven't got it to compile and there are still a few places that are yet to be figured so I'm not sure if it is even possible or not. That's why I call it experimentation and not a solution.

For the pattern you've mentioned above we can have something like this:

SemanticBuilder::with_cfg::<OxcCFG>()

Where OxcCFG is something like this:

trait CFG {
    type Cfg;
    type Builder;
}

struct OxcCFG;

impl CFG for OxcCFG {
    type Cfg = ControlFlowGraph;
    type Builder = ControlFlowGraphBuilder;
}

I'm not sure if you find it any better than the previous version so let me know what your thoughts are. I can also create a PR with my WIP code so you can give it a look, It adds a bit of complexity to the semantic build process since now we have to propagate generic arguments throughout the code but shouldn't have any impact on downstream consumers unless they decide to use the more versatile version so no added maintenance burden down stream.

[Boshen]

Let's give your version a spin. Top priority is to disable it, then iterate.