search

antihax / goesi

Go client for EVE Online ESI and SSO using versioned endpoints
MIT License
80 stars 13 forks source link

Replace rateLimiter with golang.org/x/time/rate #9

Closed tyler-sommer closed 6 years ago

tyler-sommer commented 6 years ago

Hi @antihax !

I noticed my applications were constantly using 2% CPU, so I did a little profiling with pprof and found that the rateLimiter implementation in goesi was causing the issue. Here's the top handful of rows from top100 in go tool pprof after about 30 seconds of running:

Showing nodes accounting for 600ms, 100% of 600ms total
      flat  flat%   sum%        cum   cum%
     190ms 31.67% 31.67%      190ms 31.67%  runtime.mach_semaphore_signal
     180ms 30.00% 61.67%      180ms 30.00%  runtime.mach_semaphore_wait
      80ms 13.33% 75.00%       80ms 13.33%  runtime.(*waitq).dequeue
      50ms  8.33% 83.33%       50ms  8.33%  runtime.usleep
      30ms  5.00% 88.33%       50ms  8.33%  runtime.notetsleep
      20ms  3.33% 91.67%       20ms  3.33%  runtime.mach_semaphore_timedwait
      10ms  1.67% 93.33%       10ms  1.67%  github.com/motki/cli/vendor/github.com/antihax/goesi/eveapi.(*rateLimiter).tick
      10ms  1.67% 95.00%       10ms  1.67%  runtime.acquireSudog
      10ms  1.67% 96.67%       10ms  1.67%  runtime.lock
      10ms  1.67% 98.33%       10ms  1.67%  runtime.memclrNoHeapPointers
      10ms  1.67%   100%      100ms 16.67%  runtime.startm

The very low tick rate (50ms for authedThrottle and 6.666ms for anonThrottle) caused essentially a spinwait in the tick method.

There is a "polling" rate limiter implementation in x, calculating what rate limiting needs to occur when lim.Reserve() is called. It turned out to be a drop-in replacement -- even the test still passes. This also dropped average load in my programs to 0.0%.

Let me know what you think. And as always, thanks for the excellent library!

antihax commented 6 years ago

I am actually in favor of removing the rate limiter completely. It's only use is for XMLAPI and CREST which is retiring in May. I will see if many people are still using it or not, we could just remove it this week if not.

antihax commented 6 years ago

I actually do not see anyone on github using it. They would be calling NewEVEAPIClient.

tyler-sommer commented 6 years ago

I still use the XML API for fetching corp information (hangars/divisions, blueprints, orders) but it looks like ESI now has this information available. I'd agree that its probably best to remove the XMLAPI altogether, that will also force me into upgrading :)

antihax commented 6 years ago

Gutted the code. Time to upgrade. 💯