lukeed / taskr

A fast, concurrency-focused task automation tool.
MIT License
2.53k stars 74 forks source link

Use Tinydate for timestamp #270

Closed lukeed closed 7 years ago

lukeed commented 7 years ago

This is one of my own modules. Although it's not a one-liner like I had previously, it's still only 330 bytes. However, the reason I suggest the change is that tinydate runs at 75M ops/sec, while the inline helper runs at 1M ops/sec.

Since all output is timestamped, it makes sense to make this run as cheaply & quickly as possible.

I also have the square brackets colored now:

Before:

screen shot 2017-05-03 at 9 10 37 am

After:

screen shot 2017-05-03 at 9 10 54 am
jorgebucaran commented 7 years ago

@lukeed How's tinydate faster than using toTimeString? Perhaps we don't even need toTimeString?

jorgebucaran commented 7 years ago

Good job on tinydate by the way! 👍

lukeed commented 7 years ago

@jbucaran Thanks! Because tinydate parses the template once and then returns a "render" function.

Most other formatters (including toTimeString) formats and interprets the template in the same call. Ours was performing a RegExp match on top of that, too! All slow, costly operations.

jorgebucaran commented 7 years ago

@lukeed I get it, that's very clever, but we may not need that regex at all.

What about (new Date).toLocaleDateString() or (new Date).toLocaleTimeString()?

lukeed commented 7 years ago

Even slower. In fact, I think those are the two slowest Date methods IIRC 😆

d.toLocaleDateString
  --> 188,889 ops/sec ±0.36% (94 runs sampled)
d.toLocaleTimeString()
  --> 191,945 ops/sec ±0.69% (90 runs sampled)
tinydate 
  --> 75,209,216 ops/sec ±0.70% (92 runs sampled)
jorgebucaran commented 7 years ago

@lukeed That's really fast, indeed and good to know all this stuff about the date/time methods 👏.

I like tinydate, but can't we just get away with:

$.getTime = () => {
  const pad = n => (n < 10 ? "0" + n : n)
  const d = new Date()
  return `${d.getHours()}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`
}
lukeed commented 7 years ago
newest
  -> 18,437,620 ops/sec ±0.81% (91 runs sampled)
tinydate
  --> 73,719,290 ops/sec ±1.01% (92 runs sampled)

Faster than before, yes, but not sure I understand the hesitation to use tinydate? Yours is 143b of compressed ES5. Mine is little more than double that for 4x the speed? ¯_(ツ)_/¯

jorgebucaran commented 7 years ago

It's not about the size, but the perceived added value vs adding another dependency which means I need to look into tinydate's implementation to understand what's going on when we have a simpler okayish solution already.

Sidenote: We could even get away without minimist and clor and in some not so distant future, even bluebird right?

Since you wrote tinydate, I'm sure you can improve my example above if fly needs it.

Let me be clear, I like tinydate, and I'm watching the project now to see how it evolves as I like that kind of stuff. I'm sure I'll opt for tinydate instead of moment.js when I need it, but I just don't see the added value of adding it to fly.

lukeed commented 7 years ago

I'd like to keep clor. Having pretty terminal output is a major relief.

Removing minimist is on my near todo list 👍

Bluebird has to be kept until native Promise is equally fast and memory-efficient. Right now, Bluebird puts native to shame, even on 7.x. 😢 Although, 7.4-7.9 made big improvements so I'm hopeful for 8.0! However, memory usage is still a big issue.

Can't improve your example much more. I spent a lot of last week looking at date utils & tinydate is the best-performing result of that.

I'm not accusing you of not liking it 😋 But when you're running Fly in a server environment, like I am, with 1k+ logs per min, adding tinydate definitely makes a noticeable difference. A single, npm run build isn't gunna see much benefit.

lukeed commented 7 years ago

Will use the new inline helper for now & see how it performs in the high-use environment. 👍

May open a future PR for tinydate again if not adequate.