I looked over the Common Lisp version at
https://github.com/niofis/raybench/blob/master/lisprb.lisp and it's… really bad, in a lot of ways.
(declaim (optimize (speed 3) (safety 0) (space 0) (debug 0) (compilation-speed 0)))
Never use `(optimize (safety 0))` in SBCL — it throws safety
completely out the window. We're talking C-levels of safety at that point. Buffer overruns, the works. It
might buy you 10-20% speed, but it's not worth it. Lisp responsibly, use `(safety 1)`.
(defconstant WIDTH 1280)
People generally name constants in CL with +plus-muffs+. Naming them as uppercase doesn't help because the reader uppercases symbol names by default when it reads. So `(defconstant WIDTH ...)` means you can no longer have a variable named `width` (in the same package).
(defstruct (vec
(:conc-name v-)
(:constructor v-new (x y z))
(:type (vector float)))
x y z)
Using `:type (vector float)` here is trying to make things faster, but failing. The type designator `float` covers
all kinds of floats, e.g. both `single-float`s and `double-float`s in SBCL. So all SBCL knows is that the struct contains some kind of float, and it can't really do much with that information. This means all the vector math functions below have to fall back to generic arithmetic, which is extremely slow. SBCL even warns you about this when it's compiling, thanks to the `(optimize (speed 3))` declaration, but I guess they ignored or didn't understand those warnings.
(defconstant ZERO (v-new 0.0 0.0 0.0))
This will cause problems because if it's ever evaluated more than once it'll try to redefine the constant to a new `vec` instance, which will not be `eql` to the old one. Use `alexandria:define-constant` or just make it a global variable.
All the vector math functions are slow because they have no useful type information to work with:
(disassemble 'v-add)
; disassembly for V-ADD
; Size: 160 bytes. Origin: #x52D799AF
; 9AF: 488B45F8 MOV RAX, [RBP-8] ; no-arg-parsing entry point
; 9B3: 488B5001 MOV RDX, [RAX+1]
; 9B7: 488B45F0 MOV RAX, [RBP-16]
; 9BB: 488B7801 MOV RDI, [RAX+1]
; 9BF: FF1425A8001052 CALL QWORD PTR [#x521000A8] ; GENERIC-+
; 9C6: 488955E8 MOV [RBP-24], RDX
; 9CA: 488B45F8 MOV RAX, [RBP-8]
; 9CE: 488B5009 MOV RDX, [RAX+9]
; 9D2: 488B45F0 MOV RAX, [RBP-16]
; 9D6: 488B7809 MOV RDI, [RAX+9]
; 9DA: FF1425A8001052 CALL QWORD PTR [#x521000A8] ; GENERIC-+
; 9E1: 488BDA MOV RBX, RDX
; 9E4: 488B45F8 MOV RAX, [RBP-8]
; 9E8: 488B5011 MOV RDX, [RAX+17]
; 9EC: 488B45F0 MOV RAX, [RBP-16]
; 9F0: 488B7811 MOV RDI, [RAX+17]
; 9F4: 48895DE0 MOV [RBP-32], RBX
; 9F8: FF1425A8001052 CALL QWORD PTR [#x521000A8] ; GENERIC-+
; 9FF: 488B5DE0 MOV RBX, [RBP-32]
; A03: 49896D40 MOV [R13+64], RBP ; thread.pseudo-atomic-bits
; A07: 498B4520 MOV RAX, [R13+32] ; thread.alloc-region
; A0B: 4C8D5830 LEA R11, [RAX+48]
; A0F: 4D3B5D28 CMP R11, [R13+40]
; A13: 772E JNBE L2
; A15: 4D895D20 MOV [R13+32], R11 ; thread.alloc-region
; A19: L0: C600D9 MOV BYTE PTR [RAX], -39
; A1C: C6400806 MOV BYTE PTR [RAX+8], 6
; A20: 0C0F OR AL, 15
; A22: 49316D40 XOR [R13+64], RBP ; thread.pseudo-atomic-bits
; A26: 7402 JEQ L1
; A28: CC09 BREAK 9 ; pending interrupt trap
; A2A: L1: 488B4DE8 MOV RCX, [RBP-24]
; A2E: 48894801 MOV [RAX+1], RCX
; A32: 48895809 MOV [RAX+9], RBX
; A36: 48895011 MOV [RAX+17], RDX
; A3A: 488BD0 MOV RDX, RAX
; A3D: 488BE5 MOV RSP, RBP
; A40: F8 CLC
; A41: 5D POP RBP
; A42: C3 RET
; A43: L2: 6A30 PUSH 48
; A45: FF142520001052 CALL QWORD PTR [#x52100020] ; ALLOC-TRAMP
; A4C: 58 POP RAX
; A4D: EBCA JMP L0
If they had done the type declarations correctly, it would look more like this:
; disassembly for V-ADD
; Size: 122 bytes. Origin: #x52C33A78
; 78: F30F104A05 MOVSS XMM1, [RDX+5] ; no-arg-parsing entry point
; 7D: F30F105F05 MOVSS XMM3, [RDI+5]
; 82: F30F58D9 ADDSS XMM3, XMM1
; 86: F30F104A0D MOVSS XMM1, [RDX+13]
; 8B: F30F10670D MOVSS XMM4, [RDI+13]
; 90: F30F58E1 ADDSS XMM4, XMM1
; 94: F30F104A15 MOVSS XMM1, [RDX+21]
; 99: F30F105715 MOVSS XMM2, [RDI+21]
; 9E: F30F58D1 ADDSS XMM2, XMM1
; A2: 49896D40 MOV [R13+64], RBP ; thread.pseudo-atomic-bits
; A6: 498B4520 MOV RAX, [R13+32] ; thread.alloc-region
; AA: 4C8D5820 LEA R11, [RAX+32]
; AE: 4D3B5D28 CMP R11, [R13+40]
; B2: 7734 JNBE L2
; B4: 4D895D20 MOV [R13+32], R11 ; thread.alloc-region
; B8: L0: 66C7005903 MOV WORD PTR [RAX], 857
; BD: 0C03 OR AL, 3
; BF: 49316D40 XOR [R13+64], RBP ; thread.pseudo-atomic-bits
; C3: 7402 JEQ L1
; C5: CC09 BREAK 9 ; pending interrupt trap
; C7: L1: C7400103024F50 MOV DWORD PTR [RAX+1], #x504F0203 ; #
; CE: F30F115805 MOVSS [RAX+5], XMM3
; D3: F30F11600D MOVSS [RAX+13], XMM4
; D8: F30F115015 MOVSS [RAX+21], XMM2
; DD: 488BD0 MOV RDX, RAX
; E0: 488BE5 MOV RSP, RBP
; E3: F8 CLC
; E4: 5D POP RBP
; E5: C3 RET
; E6: CC0F BREAK 15 ; Invalid argument count trap
; E8: L2: 6A20 PUSH 32
; EA: E8F1C64CFF CALL #x521001E0 ; ALLOC-TRAMP
; EF: 58 POP RAX
; F0: EBC6 JMP L0
The weirdness continues:
(defstruct (ray
(:conc-name ray-)
(:constructor ray-new (origin direction))
(:type vector))
origin direction)
The `:conc-name ray-` is useless, that's the default conc-name. And again with the `:type vector`… just make it a normal struct. I was going to guess that they were doing it so they could use vector literals to specify the objects, but then why are they bothering to define a BOA constructor here? And the slots are untyped, which, if you're looking for speed, is not doing you any favors.
I took a few minutes over lunch to add some type declarations to the slots and important functions, inlined the math, cleaned up the broken indentation and naming issues:
https://gist.github.com/sjl/005f27274adacd12ea2fc7f0b7200b80...
The old version runs in 5m12s on my laptop, the new version runs in 58s. So if we unscientifically extrapolate that to their 24m time, it puts it somewhere around 5m in their list. This matches what I usually see from SBCL: for numeric-heavy code generic arithmetic is very slow, and some judicious use of type declarations can get you to within ~5-10x of C. Getting more improvements beyond that can require really bonkers stuff that often isn't worth it.