search

goodeggs / librato-node

A Node.js client for Librato (http://librato.com/)
MIT License
37 stars 26 forks source link

Aggregator performance improvement #48

Closed keithmo closed 8 years ago

keithmo commented 8 years ago

This PR implements a performance improvement for the aggregator. Rather than storing an array of all values associated with a measure (and computing sum/min/max/etc on flush) the computed values are determined on-the-fly. This reduces memory overhead and processing time when dealing with very active measures.

Disclaimer: I'm an old grizzled software geek, but a total noob when it comes to Coffee Script. In fact, this is the very first CS I've ever written. If my changes could be written more idiomatically, please let me know.

bobzoller commented 8 years ago

looks like a nice improvement, thanks! (the coffee-script looks great too)

before I merge, please rebase and squash so we don't include your WIP commits.

keithmo commented 8 years ago

Good idea (duh) -- will do.

keithmo commented 8 years ago

Done.

bobzoller commented 8 years ago

your commits are squashed but now the rebase looks wonky. (5 commits here but we're expecting 1)

keithmo commented 8 years ago

Sorry for the delay.

I'm confused. git log shows two commits (d0d18f4ab8ccd73167ba93de5f64573843d544f9 with my changes, 75bd6336643df02c58735237fb3bef2539c01cad with "Merge branch 'master' of https://github.com/goodeggs/librato-node") but, as you point out, the Github UI says "This branch is 5 commits ahead of goodeggs:master".

Wonky indeed.

I'll ask one of the local Git gurus for advice, but if you have any suggestions I'm all ears. Thanks!

bobzoller commented 8 years ago

sorry yeah I lost track of this too. I think I can manually merge it and leave your authorship in tact. I'll try that now.

On Tue, Mar 1, 2016 at 2:55 PM, Keith Moore notifications@github.com wrote:

Sorry for the delay.

I'm confused. git log shows two commits (d0d18f4 https://github.com/goodeggs/librato-node/commit/d0d18f4ab8ccd73167ba93de5f64573843d544f9 with my changes, 75bd633 https://github.com/goodeggs/librato-node/commit/75bd6336643df02c58735237fb3bef2539c01cad with "Merge branch 'master' of https://github.com/goodeggs/librato-node") but, as you point out, the Github UI says "This branch is 5 commits ahead of goodeggs:master".

Wonky indeed.

I'll ask one of the local Git gurus for advice, but if you have any suggestions I'm all ears. Thanks!

— Reply to this email directly or view it on GitHub https://github.com/goodeggs/librato-node/pull/48#issuecomment-190948570.

bobzoller commented 8 years ago

merged in 578acee, should be released shortly as 4.1.0. thanks @keithmo!

keithmo commented 8 years ago

Thanks for your help & patience!