babel-polyfill not being required under hook::run

[zanona]

When testing the below as a hot-code:

require('babel-polyfill');
module.exports = function (h) { h.res.end(typeof regeneratorRuntime); }

It returns object, however when making it a hook and calling through the run URL, it returns: undefined.

[Marak]

That's interesting.

Why are you attempting to require babel-polyfill?

Why not just use select ES7 as your hook language and write using ES7 notation instead of having to run babel yourself?

We should already have support for this?

[zanona]

I was performing a few tests and sending an already transpiled and minified version in order to improve performance when I have noticed this.

[Marak]

Hrmm.

I'm thinking that this regeneratorRuntime variable pollutes the global scope, and our Node vm instance is unable to see it.

It might be possible to resolve by adding a single line here to pass in the global var: https://github.com/bigcompany/hook.io/blob/master/bin/run-hook#L356

The thing is, you should really just be using the ES7 hooks. If you are having other performance issues, we should address those.

[Marak]

Here you can see how we are supporting ES7: https://github.com/bigcompany/hook.io/blob/master/bin/run-hook#L74

[zanona]

Once the environment is set to production, an already transpiled version is cached or does it still performing the transform operation for every request?

[Marak]

In the code I linked you'll see a TODO saying its currently doing the transpile on every request.

We could optimize that. You are the first person to bring it up.

[zanona]

Thanks for the info @marak, I believe that would be nicer, however allowing user to provide their own version in this case would also be great, so we could make use of 'babel-polyfill' for this having better control over the issue. What do you reckon?

[Marak]

I don't like the idea of polluting every single javascript service with babel, when in the majority of the use-cases it won't be used.

The main problem is this babel tool requires pollution of global namespace.

Better you use the designated ES7 language hooks and we work on optimizing that. Either that, or find a solution which doesn't require a global regeneratorRuntime

[zanona]

yes, I agree it was bad design choice from babel making it available globablly. Perhaps theres a workaround for this, let see. Going forward, in this case it would be great to focus on improving ES7 caching. transpiling the same code over and over for every request feels like a waste of resources isnt it? so I am sure there will be a lot of benefits coming with a cached transpiled version, not only for production perhaps but as soon as the source gets updated perhaps? which I am still trying to figure out how do do :) #221

[Marak]

Related #142 - Add compile step

transpiling the same code over and over for every request feels like a waste of resources isnt it?

Yes. This wasn't an active design choice, as much as how ES7 and babel work. If it was up to me, the node binary itself would be able to understand the ES7 code natively without a transpile step.

It would be good if we understand the actual performance hit in milliseconds for transpile step. I want to prioritize our development efforts evenly across the platform.

Do you know how much of a performance hit we are incurring with transpiling every time? Logically, I'd assume a big one, but currently I have no data. Anything we can measure, we can track and improve over time.

[zanona]

Good idea, we can definitely do some benchmarks in order to measure it. In the meantime one interesting thing to look at based on babel documentation under babel-node which I believe to relate with the current behaviour:

Not meant for production use You should not be using babel-node in production. It is unnecessarily heavy, with high memory usage due to the cache being stored in memory. You will also always experience a startup performance penalty as the entire app needs to be compiled on the fly.

[Marak]

@zanona - Let me ask you. Is the current response time for ES7 hooks causing you any real production issues?

I understand that they might not be super fast, but I think for most webhook and microservice applications it should be OK?

Instead of addressing this as a node only solution, I would like to bundle this change with #142, as it seems to be the most correct thing to do.

[zanona]

Not at all, for now I have only been playing around. But figured it was a something having room for improvement. If bundling these updates together will be more beneficial I would say definitely go for it.

As I have mentioned, I would probably look for ways to compile my source code before sending to hook.io for now to make sure I get the necessary performance needed. — Just as I currently do with AWS lambda. I have stumbled upon ForbesLindesay/regenerator-runtime-only which looks promising on helping me to achieve this.

So, definitely. It would be nice to see Go around here as well. :) Having a solution that applies to all compiled languages will be certainly more efficient.

[Marak]

I just now experimented with regenerator-runtime-only again ( tried this before i think too ). Don't think it will work.

Best thing to do for sure is have toggle between Production and Development mode create a cached compiled version of any service which requires a compile step ( ES7 / Go / C++ / Swift / etc )

Once we have at least two compiled languages ready ( lets say Go and ES7 ), I'll start making an active effort to resolve #142

[zanona]

Weird, I have tried and it worked fine. However I had to append a line in the end with module.exports = exports.default; just for compatibility. Otherwise I would get:

/node_modules/run-service/index.js:17
    throw new Error('service is undefined');
          ^
service is undefined

Here's the working transpiled source:

var regeneratorRuntime = require('regenerator-runtime-only');
var exports = module['exports'];
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = function (hook) { hook.res.end('hello world'); };
module.exports = exports.default

The first 2 and the last lines are added post babel.transform

original:

export default function (hook) { hook.res.end('hello world'); }

Overall, I have also tested with async/await, etc and it's working fine so it seems.

[Marak]

Interesting. If you look at the code which handles this you'll see it can be improved.

https://github.com/bigcompany/hook.io/blob/master/bin/run-hook#L88 https://github.com/bigcompany/hooks/blob/master/gateway-es7/index.js#L32

Personally, I haven't been using ES7. I suspect there might be a few easy improvements we can make.

[Marak]

I suppose also https://github.com/bigcompany/hooks/blob/master/gateway-javascript/index.js#L14 if you are running it through Javascript gateway

[zanona]

Yes, these were exactly the ones I was basing myself in order to add the var exports = module['exports'] line in the beginning once removing the 'use strict'; from the top as well.

I will have a better look on it tomorrow and see if there is something that we can perhaps adopt in the short term for improvements. Would you think caching the transpiled version in the datastore would be recommended? If, yes, than I believe it could be even simpler to implement?

[Marak]

I think it's best to keep the transpiled version in couchdb document.

That way we get revision history.

In addition, couchdb hook document has built-in redis caching ( using datastore ) in the app, so we get the performance benefits of redis. Will revisit these code paths when I'm working on #142 .

[zanona]

Thanks. Just some really basic silly benchmarks between ES7 and transpiled version execution times:

ES7 with babel-node

time babel-node test/hook.js
real    0m2.250s
user    0m1.800s
sys     0m0.163s

Transpiled version

time node test/hook.transpiled.js
real    0m0.842s
user    0m0.421s
sys     0m0.053s

However it seems there's a fluctuation around 2 seconds for ES7 with babel execution, but still seems to take at least twice the time for my use case, of course :)

Just something to keep in mind.

[Marak]

Yeah. I think it's best to have a compile-step available for all services. As @ljharb mentioned in the IRC room, we could also add optional LINTing here.

ES7 will probably always run faster when transpiled, cached, and sent directly to node binary.

Thanks for the information and feedback. It's much appreciated.

[zanona]

Good idea. No worries, glad to help.