TrapInformation generated by mark_range_with_trap_code is too large

[slave5vw]

Thanks for proposing a new feature!

Motivation

https://github.com/wasmerio/wasmer/blob/b4d2d8ee804a45850af2b3e012fdbbbdcb1185cb/lib/compiler-singlepass/src/codegen_x64.rs#L320

In Singlepass, mark_range_with_trap_code stores per 1 byte from begin to end. However, as you know,

• instruction is not 1 byte unit, So, unnecessary offsets are included as TrapInformation.
• and can be made simpler if expressed as a range.

The problem is that the trap_table created in this way is created 1:1 as TrapInformation and is stored in the module cache. For example for one wasm, the data size created by mark_range occupied about 620KB out of module size 2.4MB.

Of course, in 1.0 the mark_range_with_trap_code caller decreased a lot. However, emit_memory_op, which occupied 90% of the capacity, still remains a problem. https://github.com/wasmerio/wasmer/blob/b4d2d8ee804a45850af2b3e012fdbbbdcb1185cb/lib/compiler-singlepass/src/codegen_x64.rs#L1357

When the size of the module increases, the problem environment is in the blockchain. https://github.com/CosmWasm/cosmwasm/issues/603#ERC20%20Performance%20comparison%20result We cached the module in-memory for faster execution. but the module size of singlepass It is a little heavy.

Proposed solution

I implemented poc of ExceptionTable with range pair key in 0.17.0. The capacity was reduced, and there were no problems running.

However, in the 1.0 code, it was refactored and confirmed that it is being used commonly as TrapInformation. So the proposal is a little vague ;). How about implement TrapInformation based on range by using the BTreemap? It looks like need to specialize for singlepass.

[webmaster128]

So you propose changing CompiledFunction's CompiledFunctionFrameInfo type from lib/compiler/src/function.rs? Because the code you linked only occupies memory during compilation, right?

[syrusakbary]

I think the solution might be as simple as replace the mark_range_with_trap_code contents with:

        let begin = self.assembler.get_offset().0;
        let ret = f(self);
        let end = self.assembler.get_offset().0;
        self.trap_table.offset_to_code.insert(begin, code);
        self.mark_instruction_address_end(end);
        ret

[webmaster128]

as simple as replace the mark_range_with_trap_code contents with

And then adapt the logic that reads the information assuming all following offests have the same trap code? Or is only the beginning relevant?

    /// Fetches trap information about a program counter in a backtrace.
    pub fn lookup_trap_info(&self, pc: usize) -> Option<&TrapInformation> {
        let module = self.module_info(pc)?;
        let func = module.function_info(pc)?;
        let traps = &module.processed_function_frame_info(func.local_index).traps;

        /// Would it still be found here? 👇
        let idx = traps
            .binary_search_by_key(&((pc - func.start) as u32), |info| info.code_offset)
            .ok()?;
        Some(&traps[idx])
    }

[syrusakbary]

I believe the logic that reads the information already supports ranges, so we would probably not need to adapt any logic there.

[slave5vw]

@webmaster128

So you propose changing CompiledFunction's CompiledFunctionFrameInfo type from lib/compiler/src/function.rs? Because the code you linked only occupies memory during compilation, right?

right,

@syrusakbary I think currently lookup_trap_info does not work with range. please look this again. Yes, already support based on range until processed_function_frame_info. However, The last TrapInformation match doesn't.

/// Would it still be found here? 👇
        let idx = traps
            .binary_search_by_key(&((pc - func.start) as u32), |info| info.code_offset)
            .ok()?;

pub struct CompiledFunctionFrameInfo {
    /// The traps (in the function body).
    ///
    /// Code offsets of the traps MUST be in ascending order.
    pub traps: Vec<TrapInformation>,

    /// The address map.
    pub address_map: FunctionAddressMap,
}

The address_map seems to serve a similar purpose at first glance, but it is not being used as a method for traps. just querying only traps when TramInformation match.

However, traps are not structured based on range. Traps are vectorized with a single offset, and range-based queries are not supported. Therefore, if only save the begin suggested above, the trap query will fail.

I think the reason for this structure is probably to be used in common with the cranelift. However, cranelift and singlepass make exceptions a bit differently. The cranelift only generates a single offset, but the singlepass, as you can see, has a range.

[slave5vw]

@losfair Looking at the list of commits, you seem leading singlepass. If it's okay, can you review this?

[whylee259]

+1

Do you have any plan to apply this issue? @syrusakbary

[ptitSeb]

Singlepass now use a trapcode embeded in the "Undefined" opcode directly (removing the table completly).

I propose to close this ticket now, are you ok with that @slave5vw ?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

Feel free to reopen the issue if it has been closed by mistake.