mozbrick / brick

UI Web Components for Modern Web Apps
http://mozbrick.github.io/
Other
2.97k stars 207 forks source link

x-tooltip slowing page to a crawl #104

Closed LukeStonehm closed 10 years ago

LukeStonehm commented 10 years ago

Ok, so I have been trying to diagnose this problem for a while.

What I am experiencing is a slow down of any page running a x-tooltip with the Hover option set. Hovering over elements causes 20% cpu spikes on a 2nd Gen i5 with 4gbs of ram. Using FF26. On my home PC I'm running FF24 and it also causes spikes. I don't see these issues on Chrome.

I profiled my page, and it led me to believe that x-tooltip was responsible. If I remove the Brick.js link my page runs full speed as it should. I have about 10 tooltips in use on the page.

I can post a dropbox link to the JSON profile data, if needed.

fwenzel commented 10 years ago

Thank you this is great feedback. @pennyfx is this something you can take a look at?

potch commented 10 years ago

@LukeStonehm I'd love to see the profile data! There's a good chance we could be causing issues here.

LukeStonehm commented 10 years ago

No problem, here is the dropbox link:

https://www.dropbox.com/s/nfka2d1gh76la86/x-tooltip.json

This data was contains the profile of me simply mousing over elements (divs, spans, tables) with a few x-tooltips (onHover) enabled.

As a side note: I'm actually using FF27 (Aurora) and FF25 on my windows PC at home. It's so hard to keep up with these version numbers. I tested last night with just the task manager and a GPU usage graph, and it seems the processor spikes are also occurring on that machine.

LukeStonehm commented 10 years ago

Is anyone taking a look at this? @potch

Right now, IE in a VM renders and runs my page faster than FF.. Which is pretty sad..

potch commented 10 years ago

@LukeStonehm thanks for the profile data! Definitely seems to be an issue with our hover code. Looking into it.

potch commented 10 years ago

@LukeStonehm can I ask you one more favor? Can you replace your brick.js with the unminified version (https://raw.github.com/mozilla/brick/master/dist/brick.js) and re-run that profile?

LukeStonehm commented 10 years ago

Ok, I have done this.

https://www.dropbox.com/s/weoe361xaf1oosl/x-tooltip-v2.json

I just wanted to note that I have been using x-datepicker and x-calander, but I see x-datepicker has been removed from the BYOB.. Is there a blog post or something about that?

potch commented 10 years ago

x-datepicker is out of the bundle temporarily while we re-vamp it. If you're actively using it, I'd be happy to put the minified/compiled code for it in the repository (source lives here: https://github.com/x-tag/datepicker)

potch commented 10 years ago

Thanks so much for the profile!

LukeStonehm commented 10 years ago

That would be nice, I am just using the last version I BYOB-ed at the moment. The site is being viewed on IE and Chrome mostly at the moment. I like FF, and dev in FF, so I am pretty much the only one noticing the slow down.

Let me know if there is anything more I can do to help, and thank you :+1:

potch commented 10 years ago

I'm working on re-vamping the way that the hover trigger style works, which will fix this. Might not be finished until next week.

LukeStonehm commented 10 years ago

That's all good, let me know if there is anything I can do to assist :+1:

pennyfx commented 10 years ago

Didn't we figure this out a few weeks ago? @potch

fwenzel commented 10 years ago

hi, @potch @pennyfx I seem to recall this is fixed?

LukeStonehm commented 10 years ago

Oh happy days, please let it be fixed :-D On Jan 16, 2014 1:48 AM, "Fred Wenzel" notifications@github.com wrote:

hi, @potch https://github.com/potch @pennyfxhttps://github.com/pennyfxI seem to recall this is fixed?

— Reply to this email directly or view it on GitHubhttps://github.com/mozilla/brick/issues/104#issuecomment-32428574 .

LukeStonehm commented 10 years ago

Any news on this?

Updated version of FF is giving me "TypeError: b is null" when I hover on any element now.

potch commented 10 years ago

@LukeStonehm We've been plagued by a number of issues with the x-tooltip in its current implementation. We want to supply a high-quality component, but it's not going to be ready for our initial 1.0. The current plan is to remove x-tooltip from the default list of Brick components, and re-introduce it in the next month with a new, more performant implementation (that will be here to stay!). The code for x-tooltip isn't gone, it lives at https://github.com/x-tag/tooltip. That's where our work is happening. If your use-case demands this component in the meantime, I'm happy to help you pull it in as a standalone component. Sorry about the delay, issues, and inconvenience.

LukeStonehm commented 10 years ago

I understand, it's no problem. I have pulled it already :) I look forward to the revamped x-tooltip :)

On Fri, Jan 31, 2014 at 12:33 AM, Potch notifications@github.com wrote:

Closed #104 https://github.com/mozilla/brick/issues/104.

Reply to this email directly or view it on GitHubhttps://github.com/mozilla/brick/issues/104 .

LukeStonehm commented 10 years ago

I see x-tooltip is back up to DL. Was this issue sorted? I am no longer seeing the errors I once was, at least on the demo page :)