janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.43k stars 221 forks source link

Move `bytes?`, `indexed?`, `dictionary?` to corelib #1244

Closed primo-ppcg closed 1 year ago

primo-ppcg commented 1 year ago

I understand that it's not desirable to move everything to corelib.c, especially things that can be implemented simply and concisely in Janet. However, for these three functions I think it may be warranted. Each of these functions compares multiple types, and are used in several places throughout boot.janet, and not only in compile-time expanded macros.

For example, in flatten:

(use spork/test)
(timeit-loop [:timeout 1] :flatten (flatten [[[1 2] 3] 4 [5 [6 7 8] 9]]))

master:

flatten 1.000s, 1.556µs/body

branch:

flatten 1.000s, 0.8098µs/body

Nearly half the time taken is spent checking the type.

For the interested, moving these functions increases the size of the executable by 128 bytes, at least on my system: master:

$ du -b /usr/local/bin/janet
797104  /usr/local/bin/janet

branch:

$ du -b /usr/local/bin/janet
797232  /usr/local/bin/janet
sogaiu commented 1 year ago

master (0ea1da80):

$ janet ~/scratch/flatten.janet
flatten 1.000s, 2.227µs/body

branch (831f41a6):

$ JANET_PATH=$HOME/.local/lib/janet ~/src/janet.primo-ppcg/build/janet ~/scratch/flatten.janet 
flatten 1.000s, 1.13µs/body

master (0ea1da80):

$ du -b `which janet`
824552  /home/user/.local/bin/janet

branch (831f41a6): (needed to install to get the binary size reduced)

$ du -b `which janet`
820584  /home/user/.local/bin/janet

The binary from the master branch was bigger by 3968 bytes here.

When comparing the branch with ecc4d80a (1.30.0), the branch was larger by 128 bytes.

v1.30.0 (ecc4d80a):

$ du -b `which janet`
820456  /home/user/.local/bin/janet

The changes didn't look like they'd have an affect on non-performance behavior, but I ran some tests on repositories anyway. As expected, I didn't notice any issues :)

primo-ppcg commented 1 year ago

We might also consider exposing lengthable?: https://github.com/janet-lang/janet/blob/0ea1da80e7e3bc3f4e148bfd73f744116049a90b/src/include/janet.h#L553-L558