Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async faster with Y.soon! #304

Merged
merged 6 commits into from
Jan 4, 2013
Merged

Async faster with Y.soon! #304

merged 6 commits into from
Jan 4, 2013

Conversation

solmsted
Copy link
Contributor

Similar to Y.later, but sooner. Y.soon standardizes the way to make something happen asynchronously but without a timed delay. Under the covers, Y.soon will call setTimeout if it must, but it will try to use the more efficient setImmediate or process.nextTick depending on the environment.

@ericf
Copy link
Member

ericf commented Oct 11, 2012

@solmsted Thanks for getting this in! I see this as a precursor to getting #241 merged in. Once we get the IE10 sprint completed, I'll work with you on putting the finishing touches and get this merged in.

@hojberg
Copy link
Contributor

hojberg commented Oct 11, 2012

this is awesome!

@rgrove
Copy link
Contributor

rgrove commented Oct 11, 2012

I'm really glad to see that most of the hacks have been removed from the browser shim, but sad to see that the postMessage hack remains.

I hate to be a naysayer, but I still feel this hack is fragile and will result in an unwarranted increase in the maintenance and testing burden of Y.soon to ensure that it doesn't break in new browser versions or uncommon usage modes.

I think this hack is unnecessary. One modern browser (IE10) already supports setImmediate(). Of the browsers that don't support it, the most widely-used ones clamp setTimeout to a 4ms minimum delay as specified in the HTML5 standard. The oldest and slowest browsers (like IE6) clamp to 10-15ms.

A 4ms delay allows for a maximum resolution of ~250hz. Even at 15ms, you get ~66hz. It's widely believed that for UI refreshes and interaction feedback, 60hz or higher is ideal. setTimeout() is capable of that.

Cases where this level of performance is unacceptable are likely to be rare in the browser, but in those specific cases a fragile hack that allows higher resolution may be warranted. I don't think this is warranted as a general-purpose utility.

I would very much like to see the postMessage hack moved to an optional module like soon-postmessage that could be required manually when desired. I don't think it should be part of the core implementation of Y.soon().

@rgrove
Copy link
Contributor

rgrove commented Oct 11, 2012

There was a lively discussion about this in IRC, in which the one thing everyone seemed to agree on was that the postMessage hack needs more research and investigation before this gets merged.

Here are a few of the open questions I have:

  1. In this jsPerf test, postMessage comes out ahead of setTimeout in total iterations per second, but what's unclear is whether this still holds true on devices with slow CPUs. At what point (if any) does the overhead of the postMessage call outweigh the savings in event processing time?
  2. Does the postMessage hack result in higher or lower (or neutral) battery usage on mobile devices? Is this enough of an issue to be a concern?
  3. postMessage is affected by same-origin restrictions. The current implementation as of this comment has a known bug that prevents it from working when executed from a file:// URL. Can this be fixed without introducing security concerns?
  4. Will the postMessage hack work reliably when YUI is used in a browser extension?
  5. Will the postMessage hack work reliably when YUI is run inside an iframe?
  6. The HTML standard specifically allows browsers to delay processing events in one task source in order to process events from another task source first. For example, a browser could choose to give events in the UI task source priority over events in the posted message task source, which would mean that postMessage might not be the fastest option depending on the event load and the particular browser. Every browser could implement this differently, and each browser's implementation could change at any time without violating the spec.

Item 6 in particular makes me think it would be unwise to assume that postMessage will always be fast just because it appears to be fast in micro-benchmarks. Browser changes, or even just different kinds of loads within the same browser, could potentially result in postMessage being unexpectedly slower in some cases.

@solmsted
Copy link
Contributor Author

This message is mostly in response to @rgrove but it's also my process of documenting and sharing some thoughts.

JsPerf has odd behavior when tests fail (which some of those are intended to) and that was old crusty test code. Here's a new test case that is much closer to what we're discussing: http://jsperf.com/postmessage-vs-settimeout

JsPerf tests speed. So far postMessage seems to be a win in speed across the board against setTimeout. Even on my old phone.

Runtime speed isn't the only concern, but I don't know how to go about testing things like battery drain, memory management, or operating system performance. I could start a test in a browser on a laptop and a phone and time how long until the battery dies, but I'm not sure if I could eliminate all of the other variables other than postMessage vs setTimeout. I'm open to suggestions as to how to test some of these things.

In my mind, runtime speed of async events has little to do with UI screen refreshing. I understand it's less common but JavaScript can be used for other stuff. Imagine you have a still frame from a 1080p video and you want to do something async to each pixel. If I've done my math right, at 250hz that will take over two hours. Based on an average of the jsPerf results so far, using postMessage would take around 45 minutes. That's still completely ridiculous for applying an image filter but it's definitely significant.

As it is now, there is a bug with the postMessage implementation where it fails to work from a file:// URL. It could be fixed by either not using it from a file:// URL or by removing the strict origin passed to postMessage. Removing the origin would mean that the guid generated for the callback could be read by another site. Is this a security issue? What could that site do with a randomly generated guid? It could try to postMessage it back to the first site to force the callback to fire, but that wouldn't do anything, there are at least four different conditions which will prevent that. The external site could know the timing of when Y.soon was being called. Is that sensitive information?

As it is now, there is a bug with the setTimeout implementation. It doesn't work at all. Chrome and Firefox throw errors when setTimeout is called with a strange context. So setTimeout will need to be wrapped in another function. The same problem might also exist for setImmediate if/when browsers implement it. The nice thing is, I can quit passing 0 to soon._asynchronizer.

I don't know much of anything about browser extensions but that's definitely a good thing to find out. Are there any other environments where YUI is expected to work?

The current implementation specifically prevents postMessage from working across frames. If that check is removed, it works fine: http://jsfiddle.net/EmyJS/. What would we want it to do? We could remove that check and just let it work based on origin. That slightly weakens security, especially for the file:// case, but I still can't think of anything harmful a potential attacker could do.

The fact that browsers and JavaScript engines change isn't new information. At this point in time, in terms of speed, we have one implementation that is better than another implementation in 100% of tested cases. Browsers could change tomorrow and completely reverse those results. It's not feasible to optimize based on what browsers could possibly do and it seems silly not to optimize anything ever because of the potential for change. When browsers change, there could be a more optimal implementation so repeat tests and change with it; its not like it's going to immediately break something.

I like the idea of developers using Y.soon and automatically getting the best performance they can get without having to ask for it. Unfortunately until setImmediate arrives in all the browsers (which they don't seem to be in a hurry about), there are pro/con trade-offs among the different implementations. The postMessage implementation seems to have the most pros and the fewest cons. I think it will work well for the majority of use cases. Yet I understand there are some very valid concerns about making it essentially the default implementation for YUI.

I believe that the value of Y.soon as a common API is far greater than the value of postMessage as a fast implementation for browsers. If it helps merge Y.soon in faster, lets move postMessage somewhere else. It could move to the gallery for optional inclusion as desired or it could move to a separate YUI module for optional inclusion as desired. Eventually, once we're more comfortable with postMessage I think it could move to a separate YUI core module for optional exclusion as desired; so it would load by default but a simple loader config would shut it down.

There's been some talk of making Y.later use Y.soon under the covers if 0 or 1 is passed in. I think I like that idea if 0 is passed in. If someone really wants to wait a millisecond for some reason, it should probably try to honor that request.

@rgrove
Copy link
Contributor

rgrove commented Oct 12, 2012

Runtime speed isn't the only concern, but I don't know how to go about testing things like battery drain, memory management, or operating system performance.

OS X's Instruments.app is fantastic for this. It will do all those things.

Imagine you have a still frame from a 1080p video and you want to do something async to each pixel. If I've done my math right, at 250hz that will take over two hours. Based on an average of the jsPerf results so far, using postMessage would take around 45 minutes. That's still completely ridiculous for applying an image filter but it's definitely significant.

I move that we strike this example from the record, because using Y.soon() to do something async for every pixel of a 1080p video frame is a horrible, horrible idea, and would still be a horrible idea even if every browser supported setImmediate().

In the first place, the way to do work like this is to use web workers. But let's assume that you can't or don't want to do that for some very good reason.

The way to do this without web workers is to chop the work up into chunks and process each chunk asynchronously. Not each pixel. A 1080p video frame contains 2,073,600 pixels. Divide them into chunks of, say, 8K, and you have 254 chunks. Process each chunk asynchronously, and while processing a chunk, just use a fast for loop to iterate over each pixel. The chunk size would need to be adjusted based on how much work you're doing and potentially how fast the client is, but by balancing chunk size vs. overall async callbacks, you can reach a happy medium that doesn't waste time in Y.soon() but still keeps the UI responsive.

In short: If you're calling Y.soon() thousands and thousands and thousands of times, you're doing it wrong. Optimizing it for this use case would be optimizing it for misuse.

The fact that browsers and JavaScript engines change isn't new information. At this point in time, in terms of speed, we have one implementation that is better than another implementation in 100% of tested cases. Browsers could change tomorrow and completely reverse those results. It's not feasible to optimize based on what browsers could possibly do and it seems silly not to optimize anything ever because of the potential for change. When browsers change, there could be a more optimal implementation so repeat tests and change with it; its not like it's going to immediately break something.

You've missed my point. Yes, browsers change. Yes, code can be updated to take changes into account.

My point is that postMessage is meant to be used for sending messages, not for scheduling async events. If browsers make subtle changes to the way postMessage events are prioritized, those changes are unlikely to be publicized and unlikely to be obvious. If, as seems to be the plan, Y.soon() becomes the basis for YUI's promises implementation, and YUI begins to rely heavily on promises, a subtle change to the behavior or performance of postMessage could have subtle but wide-ranging effects on all of YUI that would be very difficult to trace back to postMessage.

Implementing a core library feature in such a way that it relies on incidental, undocumented, and unspecified behavior of a browser API that was never intended to be used for this purpose is unwise and is likely to lead to pain and suffering.

I believe that the value of Y.soon as a common API is far greater than the value of postMessage as a fast implementation for browsers. If it helps merge Y.soon in faster, lets move postMessage somewhere else. It could move to the gallery for optional inclusion as desired or it could move to a separate YUI module for optional inclusion as desired.

👍

@solmsted
Copy link
Contributor Author

I agree, that was a poor example, I was trying to make up something right away. I think valid use cases exist for calling Y.soon a massive number of times, a game's run loop or maybe a particle simulation like this: http://jsfiddle.net/jZLKW/ but that just supports your other point. In these cases, Y.soon isn't the limiting factor of the app's performance. When Y.soon starts taking too long by itself, than multiple iterations could probably be chunked synchronously as you described. @ericf mentioned using promises for data access, including what would normally be synchronous data access like an array index lookup. If and when that is implemented, we should try to make sure it never encourages this pixel looping scenario.

@rgrove
Copy link
Contributor

rgrove commented Oct 12, 2012

@solmsted: I replaced Y.soon() with setInterval(..., 0) in your fiddle and got the same fps. So in that case at least, Y.soon() is just unnecessary code weight. Do you have an example of a use case where it's necessary? It'd probably have to be something that does very little work but with a very high frequency. I can't really think of anything off the top of my head.

A particle simulation or a game's run loop would probably be better served by requestAnimationFrame, don't you think?

@ericf mentioned using promises for data access, including what would normally be synchronous data access like an array index lookup. If and when that is implemented, we should try to make sure it never encourages this pixel looping scenario.

We definitely want promises to be fast, but I can't imagine a scenario where ~100 promises per second (which seems to be roughly the resolution of the setTimeout approach according to the jsPerf test you linked above) would be too slow. That's certainly not a limitation anyone is likely to encounter by doing data access in a browser. Perhaps more likely in Node.js, but there we already have process.nextTick().

@solmsted
Copy link
Contributor Author

@rgrove so a couple things.

requestAnimationFrame is great for rendering stuff to the screen but depending on the game or simulation, there's more work to do than that. I've played many games where NPCs will happily attack my character while I'm not paying attention using another application or browsing the web.

Yes, setInterval or setTimeout 0 yield about the same performance in that scenario, which is why I said it proves your earlier point. I don't disagree with you about postMessage, but the primary goal of Y.soon is to get people to stop using setTimeout 0 in their code. That way when setImmediate comes around, or when the code is executed in Node.js, it will automatically do the right thing. I also personally like the cancel method better than using clearTimeout. Maybe we need to make an interval version of Y.soon?

@davglass
Copy link
Member

@solmsted Make sure you do more checking for NodeJS in here. Node 0.9.x introduces setImmediate

https://github.com/joyent/node/blob/master/lib/timers.js#L314

In Node, this should always be used over process.nextTick (I couldn't find any node code in your latest sources tho).

You should update the ('setImmediate' in Y.config.win) call to also check if we are in node (Y.UA.nodejs), then check the global to see if setImmediate is available there.

Reference of how we use it in JSON:
https://github.com/yui/yui3/blob/master/src/json/js/stringify.js#L10

Side note process.nextTick was changed in node 0.9.0 to occur before all IO had completed not after. Hence the introduction of setImmediate, which is works like the old process.nextTick used to.

My other reservation on this is that we don't require or use later in core, but we bundle it for backward compat. I would rather move this to it's own module (timers maybe) and have later join it in a future release after we forecast it's removal from the seed.

@ericf
Copy link
Member

ericf commented Oct 17, 2012

You should update the ('setImmediate' in Y.config.win) call to also check if we are in node (Y.UA.nodejs), then check the global to see if setImmediate is available there.

@davglass Should we introduce a Y.config.global which normalizes this? It would point to window in the browser and global in Node.js. Also this might be more forward looking towards ES6+.

@davglass
Copy link
Member

Sounds like a good idea to me..

@lsmith
Copy link
Contributor

lsmith commented Oct 17, 2012

That would also be useful in the JSON modules to access the global JSON object.

@ghost ghost assigned ericf Oct 25, 2012
@solmsted
Copy link
Contributor Author

solmsted commented Nov 1, 2012

@davglass Thanks for the info about setImmediate in Node.js, I wasn't aware of it in 0.9.x. My current version redefines yui core to load a different module in Node.js. There's no feature detection in the Node.js version; it just always uses process.nextTick. Since setImmediate now also exists in Node.js, it no longer makes sense to split it into two different modules. Do you think it's satisfactory, in all environments, to check for the existence of setImmediate, then fall back to process.nextTick, then fall back to setTimeout? Should it ever specifically check Y.UA.nodejs or Y.UA.anything?

I was using Y.config.win for feature detection and I had a couple concerns about it, mostly involving: what happens when someone manually defines their own win object in the config or what happens when someone modifies Y.config.win at run time. Y.config.global would normalize the environments but it has these same issues. I could change feature detection to do if (typeof setImmediate === 'function') is that any better/worse than if ('setImmediate' in Y.config.global)? It should detect the global in all environments, but it won't let someone use the setImmediate off of their own custom global object config.

I'm not sure if soon and later should be defined in the same module. Especially if we ever want to make Y.later(0, ...) use Y.soon, it feels cleaner to keep them separate. That way, it's just a simple loader config to hack in a replacement soon module. 'timers' might still be helpful as a virtual rollup. Anyway, I think it makes sense to pull soon out of core if later leaves too.

In summary, I'm going to start working on the following changes:
Make soon it's own module, apart from core.
Merge the Node.js and non-Node.js versions into one common module.
The postMessage asynchronizer will be available, but users will have to opt-in somehow.

I'm also looking for a convenient way to make multiple asynchronizers available side-by-side. When setImmediate isn't available, I would personally tend to use postMessage for most things, but there are occasions where it isn't the best option. There are also very good use cases for using a DOM mutation observer as an asynchronizer but I feel like it's a poor choice for any use case that doesn't touch the DOM within the callback. So far, everything I've come up with to accomplish this has been way too complicated.

@ericf
Copy link
Member

ericf commented Nov 1, 2012

Do you think it's satisfactory, in all environments, to check for the existence of setImmediate, then fall back to process.nextTick, then fall back to setTimeout? Should it ever specifically check Y.UA.nodejs or Y.UA.anything?

Yes, I do; it's not worth the module overhead.

I was using Y.config.win for feature detection and I had a couple concerns about it, mostly involving: what happens when someone manually defines their own win object in the config or what happens when someone modifies Y.config.win at run time. Y.config.global would normalize the environments but it has these same issues. I could change feature detection to do if (typeof setImmediate === 'function') is that any better/worse than if ('setImmediate' in Y.config.global)? It should detect the global in all environments, but it won't let someone use the setImmediate off of their own custom global object config.

We landed Y.config.global in core, so you can use that.

I'm not sure if soon and later should be defined in the same module. Especially if we ever want to make Y.later(0, ...) use Y.soon, it feels cleaner to keep them separate. That way, it's just a simple loader config to hack in a replacement soon module. 'timers' might still be helpful as a virtual rollup. Anyway, I think it makes sense to pull soon out of core if later leaves too.

It seems that it would be easier to support Y.later(0, ...) using Y.soon() if they were defined in the same module. I think this depends on the size of think the soon module will be. If we end up with something relatively small, I like the idea of Y.later() and Y.soon() being provided by the same, core, module.

In summary, I'm going to start working on the following changes:
Make soon it's own module, apart from core.
Merge the Node.js and non-Node.js versions into one common module.
The postMessage asynchronizer will be available, but users will have to opt-in somehow.

I think (2) makes sense no matter what. Point (1) depends on the decision of (3). We came to agreement that @rgrove's questions must be answered before we include the postMessage implementation. As we sit today, we don't have answers to these questions.

I'm also looking for a convenient way to make multiple asynchronizers available side-by-side. When setImmediate isn't available, I would personally tend to use postMessage for most things, but there are occasions where it isn't the best option. There are also very good use cases for using a DOM mutation observer as an asynchronizer but I feel like it's a poor choice for any use case that doesn't touch the DOM within the callback. So far, everything I've come up with to accomplish this has been way too complicated.

I would also like to remove the idea of implementation choice from this feature. We should not make developers decide, instead we should make the best choice for them. This public API of this feature is dead simple, and would only be made overly complex if we introduce choice.

To me this now comes done to a matters of schedule, confidence, and dependents…

@lsmith is continuing to collaborate with other library developers to refine a common promises feature set and behaviors while removing ambiguities in the spec(s). While I think it is still possible that we can get promises to land in YUI 3.8.0, I'd rather us get it right, than rush it; making 3.9.0 a more likely release where we'll land Y.Promise. Promises are the main feature dependent on Y.soon().

Currently, we have unanswered questions about the postMessage implementation, and we can only feel confident in this being the default fallback if these questions are answered. The current 3.8.0 release date is December 4th, if we can't get the postMessage questions answered soon, then it looks like we should drop it for the first release of Y.soon(). @solmsted I understand that answering these questions is not an easy task, and you should definitely not feel like it's something you have to do. If those challenges greatly interest you, then I say go for it because this information will be valuable to the web developer community at large.

At this point, I'm in favor of landing something we are fully confident will work. Even the simplest implementation of Y.soon() will be valuable, and I'd rather us error on the side of simple API with a robust implementation, than speedy and extensible one.

@solmsted
Copy link
Contributor Author

solmsted commented Nov 3, 2012

@ericf I should have explained better. I'm thinking about how to support different asynchronizers side-by-side but the other asynchronizers and the interface to invoke them will probably end up in a gallery module and not in YUI. In the current implementation of Y.soon, there's no way to accomplish this without completely redefining Y.soon in a custom module. I'd like for it to be possible to write a custom module that enhances Y.soon without redefining it. This may or may not be worth pursuing further, Y.soon itself is pretty small. I was just thinking out loud about it.

@ericf and @davglass are giving me conflicting opinions about whether soon should or should not be a core module. I don't feel qualified to resolve this myself and I haven't investigated the extent of the repercussions either way. I do feel like soon and later should either both be in core or both not be, so the first decision might need to be: what's happening with Y.later? Also if Y.Later is going to change in any way, I'd like to vote for the addition of a lighter version that doesn't handle context or arguments.

I still think soon should be a module by itself. My only reason for this is so {condition: {trigger: 'soon', when: 'instead'}} will allow a custom module to replace it, and that custom module doesn't have to redefine Y.later also. If soon is not part of core, than doing this will also save some download bytes. The only down side is that the loader metadata and combo urls will grow one module bigger. The primary use case where I foresee this being helpful is when a custom module replaces Y.soon with an enhanced implementation, which as mentioned, I'm intending to write at some point in the future. This is my back up plan for implementing that module if I can't figure out any other simple solution. Also, if for some reason we end up keeping postMessage against @rgrove's wishes, he could use this to fix it however he wants in his projects.

As much as I like the gallery-soon approach of automatically polyfilling the fastest implementation for the given environment, I think that the discussion around postMessage has proven that we can't completely take developer's implementation choice out of the equation. So far I think we all agree on setImmediate > process.nextTick > setTimeout 0. Unfortunately, in most environments the first two don't exist so setTimeout 0 automatically becomes the most common implementation. At the time I wrote gallery-soon, the premise was, setTimeout 0 is absolutely the worst way to implement this; do everything possible to avoid using it. I backed up this premise with jsperf speed results and ended up with setImmediate > process.nextTick > postMessage > messageChannel > DOM mutation observer > onreadystatechange > setTimeout 0. I considered it a success when it worked in every environment I tried it in and it never fell back to setTimeout 0. @rgrove has now brought this original premise into question and the speed results alone are no longer satisfactory to support it. Is postMessage better than setTimeout 0? At this time there are some unknowns which prevent a definite answer to that question. I'm willing to make a decision for my own projects, but I'm unwilling to make the decision for all projects using YUI. My assumption is that all of these unknowns will never be eliminated in a definitive way, therefore every developer will need to make their own judgement call for their own project. If my reasoning is accurate so far, this all reduces down to one question: should developer's have to opt-in to using postMessage or opt-out from it. I feel like for the vast majority of use cases, developers won't care or notice a difference either way, so they won't bother to opt in or out. Without more information to go on, the most compelling reason to go one way or the other is to appease @rgrove by making postMessage an opt-in feature. It makes sense, since there will be other asynchronizers available by opt-in in the gallery.

I've already answered and half-answered some of @rgrove's questions. I'll summarize here:

1: postMessage comes out ahead of setTimeout in total iterations per second, but what's unclear is whether this still holds true on devices with slow CPUs. At what point (if any) does the overhead of the postMessage call outweigh the savings in event processing time?

I've never seen postMessage lose in speed against setTimeout 0 even when scaling down to the processor on my old Android phone. I realize that's no where near definitive, but it should cover the majority of processors in use today. This is worth testing further, but I don't have any more devices to test with. http://jsperf.com/postmessage-vs-settimeout My hypothesis is that the overhead of creating a timer will always be greater than the overhead of pushing a new task to a message queue.

2: Does the postMessage hack result in higher or lower (or neutral) battery usage on mobile devices? Is this enough of an issue to be a concern?

This is a tricky question. It's already been shown that postMessage is capible of running more iterations per second. My guess is that if postMessage and setTimeout were running as fast as possible for a given length of time, postMessage would drain the battery further because it was doing more stuff. My guess is that if postMessage and setTimeout were both run a fixed number of times spread throughout a given length of time, that postMessage would drain the battery much less because it doesn't have to use timers. These are just guesses. I would also guess that the results would be drastically different depending on device hardware, operating system, browser, and other running processes. With all of these variables, I'm not sure how to conduct a meaningful experiment. I think battery drain is worth paying attention to but it's difficult to determine how much this one utility function will contribute to the battery drain of the average application.

3: postMessage is affected by same-origin restrictions. The current implementation as of this comment has a known bug that prevents it from working when executed from a file:// URL. Can this be fixed without introducing security concerns?

It can be fixed and at this time I personally can't think of any real security concerns. When I get some time, hopefully this weekend, I'll post an example somewhere and we can all attack it and see if we can compromise it. Is there any better way to test security strength?

4: Will the postMessage hack work reliably when YUI is used in a browser extension?

At this time I have no idea and I don't know how to go about creating a browser extension to test it. I don't know how the environment within the extension differs from anything else. If the extension's environment is different in any significant way, extensions aren't mentioned here http://yuilibrary.com/yui/environments/ so I would assume that YUI doesn't support running in an extension. That's probably not the answer that any of us want. If postMessage fails in any way within an extension, I would either try to fix it, or fix the polyfill so Y.soon doesn't use postMessage in that environment. Someone with more knowledge about extensions will need to help us out with this.

5: Will the postMessage hack work reliably when YUI is run inside an iframe?

The current implementation specifically prevents postMessage from working across frames. I'm thinking it's better to remove this artificial constraint. Then we'll test security along with # 3.

6: The HTML standard specifically allows browsers to delay processing events in one task source in order to process events from another task source first. For example, a browser could choose to give events in the UI task source priority over events in the posted message task source, which would mean that postMessage might not be the fastest option depending on the event load and the particular browser. Every browser could implement this differently, and each browser's implementation could change at any time without violating the spec.

This isn't a question but this is an accurate statement. The speed boost we see in browsers today may increase or decrease at any time in any browser. We can neither predict nor prevent this from happening. I'd be willing to trust that browser venders aren't going to let the message queue back up for too long, so in any case Y.soon isn't going to suddenly break.

@juandopazo
Copy link
Member

FWIW +1 to landing the simplest implementation possible, without an extra module, with the lower byte count possible.

@solmsted
Copy link
Contributor Author

solmsted commented Nov 4, 2012

This is my favorite version so far: https://gist.github.com/4009539

It's a non-@YUI_CORE@ module with about 30 lines of code. Nothing terribly complex or controversial, and there's just enough wiggle room for developers to hack in alternate implementations. If you look back at my forum post, this is pretty much what I originally wanted to do.

@juandopazo
Copy link
Member

Very much relevant: promises-aplus/promises-spec#42. It turns out timers are turned off in hidden tabs in Mobile Safari.

@solmsted
Copy link
Contributor Author

@juandopazo I haven't been following the new promises specs but that's very interesting that others are recommending the use of postMessage. Ugh, just when I thought that we had maybe reached a consensus on what to do with postMessage.

If this is true, Mobile Safari is going to be breaking lots of applications in strange ways when users switch tabs. Should Y.soon use conditional logic to attempt to avoid such breakage or should we continue to let developers decide for themselves when and how to opt in to alternate implementations?

@juandopazo
Copy link
Member

Also relevant: https://gist.github.com/2802407

@solmsted
Copy link
Contributor Author

I couldn't figure out a clean way to update the pull request with the correct code and pull in upstream changes and move the request to the dev-master branch. Instead, I did a couple of git reverts and committed my new version.

I can close this pull request and issue a new one if that would help in any way.

@ericf
Copy link
Member

ericf commented Dec 10, 2012

There's no need to create another Pull Request, we'll handle merging it into the correct branch (dev-3.x).

@ghost ghost assigned derek and davglass Jan 3, 2013
@jenny
Copy link
Member

jenny commented Jan 3, 2013

As per 1/3/13 Roundtable discussion, we'd like to include Y.soon in a timers module. Eventually we'd like to move Y.later out of core into that module also, but only after following the proper deprecation path. Reassigning to @davglass to handle that reorg.

@solmsted
Copy link
Contributor Author

solmsted commented Jan 4, 2013

My last commit just renamed the module to timers and left everything else alone. So now there's a new module, not part of @YUI_CORE@, it's called timers, and it defines Y.soon and nothing else. Y.later was always so slow...it can come join the timers module in the future.

I'm fine with this solution because we're finally moving forward. For the record, there are two very minor things I dislike about this:

  • One of the goals of Y.soon was to prevent the use of a timer when it's not necessary, so it feels wrong to put it in a module called timers. Since this naming matches Node.js, I guess it's acceptable.
  • When soon was a module all by itself it was easier to use conditional loading to replace the whole thing with an alternate implementation. Now alternate implementations will either have to redefine Y.soon and incur the slight overhead of downloading the timers version, or use conditional loading and implement everything that's packaged in the timers module.

@davglass
Copy link
Member

davglass commented Jan 4, 2013

@solmsted I'm fine with both of those isses:

  1. It's a timer no matter how you look at it ;)
  2. That's the proper thing to do, before it was overly complex now it's simple. Replace the module and redefine Y.soon as any other override module would do.

@davglass
Copy link
Member

davglass commented Jan 4, 2013

Merged into dev-3.x, I did a little cleanup on the module, History, README, tests and fixed the single lint warning. This should auto close after the next build.

@yuibuild yuibuild merged commit 6ffd57f into yui:3.x Jan 4, 2013
@solmsted
Copy link
Contributor Author

solmsted commented Jan 4, 2013

Thanks Dav!

@solmsted solmsted deleted the soon branch January 4, 2013 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants