ysmood / yaku

A lightweight promise library
https://tonicdev.com/ysmood/yaku
MIT License
291 stars 28 forks source link

Something strange (Slow resolve on pageload) #41

Closed katywings closed 7 years ago

katywings commented 7 years ago

Hello hello

First things first, i like your library :heart: :hear_no_evil: , it provides huge benefits compared to other promraries (promise-libraries :smile:)! Mainly its size is a really killer feature! But the longStackTraces are a good thing too.

But now I found a very strange problem: As soon as I use Yaku with rollup or brunch (yes I tried both), I notice a very long wait time from resolve to then. It is loading and loading, while a bluebird promise which was started in the same time as yaku, already has resolved.

dec 25 2016 10-14 pm - edited

(When I try yaku promises after the page is fully loaded, it works without problems, this slow resolve only happens on pageload)

Do you have an idea?

This is the code:

Yaku.resolve().then(function() {
  console.log('yaku1')
})

Yaku.resolve().then(function() {
  console.log('yaku2')
})

bluebird.resolve().then(function() {
  console.log('bluebird')
})

One thing I noticed too is, that when I console.log(Yaku) this log takes the same time to complete as the Yaku resolves! It is very strange: first the log just outputs function, and then it magical changes the function to:

function Promise(executor) {
        var self = this,
            err;

        // "this._s" is the internal state of: pending, resolved or rejected
        // "this._v" is the internal value

       …

This is the rollup config I used:

var flow = require('rollup-plugin-flow');
import babel from 'rollup-plugin-babel';
var serve = require('rollup-plugin-serve')
var path = require('path')
import nodeResolve from 'rollup-plugin-node-resolve';
import commonjs from 'rollup-plugin-commonjs';

export default {
  entry: 'app/main.js',
  dest: 'app/assets/bundle.js',
  plugins: [ flow(), nodeResolve({
        jsnext: true, main: true
      }), commonjs(), serve({
    contentBase: path.join(__dirname, 'app', 'assets')
      }) ],
  format: 'umd'
};
ysmood commented 7 years ago

Thanks for your feedback, I will take some time to find out what's going on there.

ysmood commented 7 years ago

First, I suggest you to use this and try again: https://github.com/ysmood/yaku#yakunexttick Maybe the problem is because Yaku use setTimeout by default, and it's slow.

var Promise = require('yaku');
require('setimmediate')
Promise.nextTick = fn => window.setImmediate(fn);
ysmood commented 7 years ago

By the way, can I have you package.json and app/main.js, I need to reproduce your issue.

katywings commented 7 years ago

Thank you for your fast feedback! And that even in christmas time! :beers:

setimmediate speeded it up greatly, but its interestingly still slower than bluebird.

Here is the testing-environment :woman_technologist: bug.zip


Okay I tried it with an other nexttick library (https://github.com/medikoo/next-tick). This library had a better performance than setImmediate and the yaku logs showed up in the correct order (yaku before bluebird)

import bluebird from 'bluebird'
import Yaku from 'yaku'

//import setImmediate from 'setimmediate'
//
import nextTick from 'next-tick'
Yaku.nextTick = fn => nextTick(fn);

console.log(Yaku)

Yaku.resolve().then(function() {
  console.log('yaku1')
})

Yaku.resolve().then(function() {
  console.log('yaku2')
})

bluebird.resolve().then(function() {
  console.log('bluebird')
})

I think it would be very important to atleast show the nextTick info in the README at a more noticable position! What were your reasons to not include a nextTick in yaku? Size? Trust? :) next-tick is 1.86 KB-ish unminified :D

ysmood commented 7 years ago
ysmood commented 7 years ago

If next-tick is good enought, and currently we are sure it's the best in the industry, I think I will make it the default option. For now, I'll just update the documentation, because it's a little better than setimmediate (next-tick is not that good for old IEs)

katywings commented 7 years ago

Do however you like its your library! ;), (https://npms.io/search?q=nextTick) the score of next-tick is not too bad though!

When the setTimeout is slow, it means the UI is slowed down by intense CPU workload, such as the first page rendering. For your case, even though bluebird is faster, there no actual meaning to be fast when the UI is slow, the bottlenect is on the UI rendering workload

What do you mean with UI rendering? Rendering of html? Or just the UI of the browser?

katywings commented 7 years ago

Wait :/, i noticed something... -.-, as soon as a remove the line import bluebird from 'bluebird' the slow loading time shows up again :hankey:

I think bluebird polyfills something which then is used by the nextTick polyfill xDDD, will look into it and give you feedback later ;)

katywings commented 7 years ago

I tried to find something in bluebird, which looks like a nextTick or a polyfill and could only find this info:

https://github.com/petkaantonov/bluebird/wiki/Error:-No-async-scheduler-available

I've spent now many hours to solve this issue and couldnt find a solution without bluebird :]... So i decided to just use bluebird core as a temporary workaround :/.

ysmood commented 7 years ago

That will be a bad sign if so, because it means bluebird silently mutates some global function. But I think it's not what really happens.

So again please show me your project details, such as the "package.json" and "main.js"

katywings commented 7 years ago

I already did 17 hours ago

Here is the testing-environment :woman_technologist: bug.zip

(https://github.com/ysmood/yaku/files/672920/bug.zip)

ysmood commented 7 years ago

Oh, so sorry for my blindness.

katywings commented 7 years ago

No problem, oh and you can run it (after install) with rollup -c .rolluprc

ysmood commented 7 years ago

Bluebird

import Bluebird from 'bluebird'
console.time('bluebird')

Bluebird.resolve().then(function() {
  console.timeEnd('bluebird')
})

Bluebird.resolve().then(function() {
  console.timeEnd('bluebird')
})

Result:

bluebird: 1.451ms
bluebird: 4.895ms

Yaku

import Yaku from 'yaku'

import nextTick from 'next-tick'
Yaku.nextTick = nextTick

console.time('yaku')

Yaku.resolve().then(function() {
  console.timeEnd('yaku')
})

Yaku.resolve().then(function() {
  console.timeEnd('yaku')
})

Result:

yaku: 1.301ms
yaku: 3.738ms

Everything seems well, I cannot reproduce your issue.

katywings commented 7 years ago

Was the bluebird test in the same file as the yaku test?

ysmood commented 7 years ago

Sure, no. These are two separate entry files.

ysmood commented 7 years ago

To make sure everything is clean, I use browser's incognito mode without any browser extension.

ysmood commented 7 years ago

Maybe your rebuild process isn't correct. This is the bundled file, you can check the source code, no bluebird insdie:

bundle.js.zip

katywings commented 7 years ago

Kk, i tried it a moment ago on a different machine and got similar results like you. Will do some last checks on the original machine this evening. Maybe its really something wrong in the rebuild process, some cache or whatever ^^.

katywings commented 7 years ago

Good news! I found the problem, it was in my build process, complicated to give you a short description, but basically next-tick was packaged by webpack in a way like this:

function(process, setImmediate....) {
   ...nextTick...
}.call(shim, shim, shim, holy moly

Therefore nextTick just used webpacks setImmediate shim, which currently seems to be slow: https://github.com/webpack/webpack/issues/947

This only happend on the parts of the application, which were built by webpack. Yaku/nextTick, directly imported in rollup didn't had the problem.

I will probably remove all of the webpack stuff from my build process and just go to rollup ;)


Behind the scenes: I use yaku in a state management library Alo, and that is currently built by webpack. And this issue #41 first occured only when I worked on a example for alo (separate repository), which was built using brunch. Thus it was very tricky to find the real root reason for the slow pageload :(.

Thx for your help :cake: !


If its okay for you, I will give you a closing-comment when I migrated/fixed the build process, and did a last test.

ysmood commented 7 years ago

Great work! And it did remind me the weird behavior of webpack.

You can use this http://webpack.github.io/docs/configuration.html#node to disable webpack's process shim, it's enabled by default. In my experience, most times it only causes confusing.

ysmood commented 7 years ago

By the way, Yaku had fast nextTick one year before, checkout the old version: https://github.com/ysmood/yaku/blob/59bb6511c27c8a262981685bed8ead4dc3569923/src/yaku.coffee#L350

I removed it a year ago to keep myself focused, but now I may revert it back.

katywings commented 7 years ago

Thanks for the idea with the webpack configuration, i finally switched to rollup though :).

Now it works like a charm, and my lib is at 67 kb thanks to your lite approach :dancer: