capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.
http://www.capstone-engine.org
7.18k stars 1.52k forks source link

Replace cs_insn.bytes with an union. Breaking API change. #2375

Closed numas13 closed 1 month ago

numas13 commented 1 month ago

Your checklist for this pull request

Detailed description

Using fixed size arrays for instruction bytes is very good for performance and simplicity but it has some drawbacks. It can not be used for achitectures with instructions longer than fixed size array for instruction bytes. But big arrays is not practical for architecutres with small instructions.

This patch changes cs_insn.bytes into an union of small fixed array for small instructions and a pointer to dynamically allocated memory for very long instructions.

The size of the fixed array is chosen to be 16 bytes based on x86 instruction lengths.

Benchmarks

old is https://github.com/capstone-engine/capstone/pull/2367. libc.so is a standard C library from my amd64 machine.

❯ hyperfine -N -r5 ./old/test_iter_benchmark ./new/test_iter_benchmark                                                                                                                                         
Benchmark 1: ./old/test_iter_benchmark
  Time (mean ± σ):     10.830 s ±  0.075 s    [User: 10.771 s, System: 0.004 s]
  Range (min … max):   10.706 s … 10.884 s    5 runs

Benchmark 2: ./new/test_iter_benchmark
  Time (mean ± σ):     10.941 s ±  0.146 s    [User: 10.927 s, System: 0.003 s]
  Range (min … max):   10.722 s … 11.060 s    5 runs

Summary
  './old/test_iter_benchmark' ran
    1.01 ± 0.02 times faster than './new/test_iter_benchmark'

❯ ARGS="10 0x028800 0x1517dd libc.so"; hyperfine -N "./old/test_file_benchmark $ARGS" "./new/test_file_benchmark $ARGS"                                                                                        
Benchmark 1: ./old/test_file_benchmark 10 0x028800 0x1517dd libc.so
  Time (mean ± σ):      2.392 s ±  0.091 s    [User: 2.354 s, System: 0.003 s]
  Range (min … max):    2.286 s …  2.621 s    10 runs

Benchmark 2: ./new/test_file_benchmark 10 0x028800 0x1517dd libc.so
  Time (mean ± σ):      2.414 s ±  0.039 s    [User: 2.404 s, System: 0.004 s]
  Range (min … max):    2.342 s …  2.467 s    10 runs

Summary
  './old/test_file_benchmark 10 0x028800 0x1517dd libc.so' ran
    1.01 ± 0.04 times faster than './new/test_file_benchmark 10 0x028800 0x1517dd libc.so'

If bytes_arr size is 8 bytes.

❯ hyperfine -N -r5 ./old/test_iter_benchmark ./new/test_iter_benchmark
Benchmark 1: ./old/test_iter_benchmark
  Time (mean ± σ):     10.745 s ±  0.132 s    [User: 10.721 s, System: 0.006 s]
  Range (min … max):   10.643 s … 10.967 s    5 runs

Benchmark 2: ./new/test_iter_benchmark
  Time (mean ± σ):     10.814 s ±  0.192 s    [User: 10.786 s, System: 0.007 s]
  Range (min … max):   10.566 s … 11.064 s    5 runs

Summary
  './old/test_iter_benchmark' ran
    1.01 ± 0.02 times faster than './new/test_iter_benchmark'

❯ ARGS="10 0x028800 0x1517dd libc.so"; hyperfine -N "./old/test_file_benchmark $ARGS" "./new/test_file_benchmark $ARGS"                                                         
Benchmark 1: ./old/test_file_benchmark 10 0x028800 0x1517dd libc.so
  Time (mean ± σ):      2.376 s ±  0.093 s    [User: 2.369 s, System: 0.004 s]
  Range (min … max):    2.301 s …  2.622 s    10 runs

Benchmark 2: ./new/test_file_benchmark 10 0x028800 0x1517dd libc.so
  Time (mean ± σ):      2.422 s ±  0.042 s    [User: 2.414 s, System: 0.003 s]
  Range (min … max):    2.372 s …  2.513 s    10 runs

Summary
  './old/test_file_benchmark 10 0x028800 0x1517dd libc.so' ran
    1.02 ± 0.04 times faster than './new/test_file_benchmark 10 0x028800 0x1517dd libc.so'

Test plan

Update bindings and user software.

Closing issues

...

aquynh commented 1 month ago

this breaks everything. can you come up with some compromise, so old apps still work?

numas13 commented 1 month ago

I'm closing PRs. We'll think about what can be done.