losvedir / transit-lang-cmp

Programming language comparison by reimplementing the same transit data app
MIT License
426 stars 31 forks source link

Elixir: use ets select instead of lookup loop #9

Closed ckampfe closed 2 years ago

ckampfe commented 2 years ago

Hey thanks for the cool comparison project, I love stuff like this.

This PR uses :ets.select and matchspecs to avoid doing :ets.lookup in a Enum.map loop. Basically getting rid of an N+1 style lookup.

I benchmarked this by waiting for the data to finish loading (as confirmed by curl) and then running the k6 commands as specified in the README, and results look like this for loadTest.js, which are pretty different from the results I was getting on the unchanged project:

at [ 16:26:27 ] ➜ k6 run -u 50 --duration 30s loadTest.js

          /\      |‾‾| /‾‾/   /‾‾/
     /\  /  \     |  |/  /   /  /
    /  \/    \    |     (   /   ‾‾\
   /          \   |  |\  \ |  (‾)  |
  / __________ \  |__| \__\ \_____/ .io

  execution: local
     script: loadTest.js
     output: -

  scenarios: (100.00%) 1 scenario, 50 max VUs, 1m0s max duration (incl. graceful stop):
           * default: 50 looping VUs for 30s (gracefulStop: 30s)

running (0m31.8s), 00/50 VUs, 371 complete and 0 interrupted iterations
default ✓ [======================================] 50 VUs  30s

     data_received..................: 1.8 GB 55 MB/s
     data_sent......................: 3.4 MB 109 kB/s
     http_req_blocked...............: avg=7.07µs  min=0s    med=1µs     max=11.06ms p(90)=3µs      p(95)=3µs
     http_req_connecting............: avg=1.86µs  min=0s    med=0s      max=1.92ms  p(90)=0s       p(95)=0s
     http_req_duration..............: avg=42.15ms min=116µs med=25.32ms max=417.4ms p(90)=103.19ms p(95)=135.44ms
       { expected_response:true }...: avg=42.15ms min=116µs med=25.32ms max=417.4ms p(90)=103.19ms p(95)=135.44ms
     http_req_failed................: 0.00%  ✓ 0           ✗ 36729
     http_req_receiving.............: avg=85.41µs min=5µs   med=57µs    max=17.33ms p(90)=144µs    p(95)=200µs
     http_req_sending...............: avg=16.71µs min=1µs   med=6µs     max=22.81ms p(90)=13µs     p(95)=17µs
     http_req_tls_handshaking.......: avg=0s      min=0s    med=0s      max=0s      p(90)=0s       p(95)=0s
     http_req_waiting...............: avg=42.05ms min=107µs med=25.2ms  max=416.7ms p(90)=103.09ms p(95)=135.3ms
     http_reqs......................: 36729  1155.153839/s
     iteration_duration.............: avg=4.17s   min=1.83s med=4.23s   max=5.99s   p(90)=5.18s    p(95)=5.3s
     iterations.....................: 371    11.668221/s
     vus............................: 26     min=26        max=50

I'm not exactly sure how to test the json results from the API (and this was a pretty quick change) so it's entirely possible I've just broken the API and this benchmark is total garbage, but maybe you can help me see if the results I'm getting are what you expect the endpoint to return. In any case thanks again, and maybe this is useful!

losvedir commented 2 years ago

Hot dog I'm excited to try this! I knew there was an n+1 thing going on and spent a bit of time trying to wrangle :ets.select before throwing in the towel on the matchspec. Of course a list of matches is the way to go. I was trying to build up a bunch of orelses in a single guard, doh.

Thanks!

ckampfe commented 2 years ago

@losvedir you may want to hold off on merging this, I'm still trying to make sure these results are repeatable

ckampfe commented 2 years ago

@losvedir Heh, after attempting to repeat these results, I'm not able to get similar results to the original, so I must have a made a mistake in benchmarking somehow. It seems like this change isn't having the effect I thought it was. That said, I'm confused, and curious why select doesn't appear to be working how I thought it would. I'm wondering, what is the select BIF doing under the hood? Is it basically a lookup loop? I have no idea! Something to look into! 😂

In any case feel free to do with this PR as you wish. I'll open a different one if I come up with something, sorry for the distraction 😂

losvedir commented 2 years ago

This is fascinating to me! I took a stab at it myself while you were working on it (once I saw the light with your generation of the match via the for(...) expression) and I was sure it would be a big win! But I'm also seeing actually worse performance with this approach, for reasons I don't understand.

I'm closing the PR for now since the experiment didn't pan out. But in any case, thanks for opening it! This was something on my to-do list to figure out, so I'm glad to have explored it a bit.