jQuery Forum

Very interesting patch - sorry I apparently missed it/forgot about it
before. I'm loving the speed improvements (loaded up IE7 and I'm
seeing 2x+ improvements across the board - along with Firefox, etc.)
There are a bunch of points though that we'll need to take into consideration:
1) This isn't actually a patch right now - it's more of a plugin (a
patch would modify the existing codebase and provide a diff of the
changes that need to be made).
2) The code, as it stands, doesn't match any of the existing coding
style of jQuery (braces not matching, $ used instead of jQuery, code
on the same line as an if, not enough spaces around statements).
3) Why does $.size exist? Why not just have the functionality be in
.height()/.width() - or in .curCSS(). I don't see a need for a new
function here.
4) Does it still pass the test suite?
Any progress on this would definitely help to get this patch landed. I
can take a look at reorganizing all of it eventually - but it might
just get delayed - you help will make it go faster.
Thanks!
--John

+1, nice work mike! 

On Fri, 2009-02-13 at 16:25 -0500, John Resig

John, thanks for clarifying those points for me. I am learning as I
go.
1) I will need help with the patch/diff business, as I have no idea
how to do any of that. Though I would like to learn.
2) I can fix the coding style (I think).
3) I used the new function "size" because I thought it was better...
The current version of jQuery uses a macro and puts much of the
duplicate logic in both the height and width methods. I though by
combining the code into a single shared method, it would use less
memory and maybe be faster. Also, because it is a static method, it
could be called in other places (jQuery.css) directly on elements.
4) I think it will pass the test suite, but again, something I would
need some help with...
-mike

> On Fri, 2009-02-13 at 16:25 -0500, John Resig

Yes, still need to show the element if it is hidden, but instead of
checking for ":visible" just checking for "display:none"
The if statement passes through values that you might be setting. 500
or '400px' or "100%"

On Fri, 2009-02-13 at 17:10 -0500, John Resig

If you mean another additional method, I could see some merit in that.
If you mean to calculate both the height and width every time you call
height or width, I think that would do more negative impact than good.

On Fri, 2009-02-13 at 14:28 -0800, mike.helgeson

Ok, so I took some time and read through the patch more completely. It
didn't seem like much had actually changed so I wanted to figure out
the differences. The performance comes from two places:
1) Not using .is(":visible") (that speeds up height/width).
2) Re-organizing inner/outerWidth so that they could fast-path through
the height/width calculations without having to have the dimension be
re-added.
So, I took those two concept and integrated them.
The perf test suite:
http://dev.jquery.com/~john/ticket/3082/
The test suite (passing in all browsers):
http://dev.jquery.com/~john/ticket/3082/test/?eight (height tests)
http://dev.jquery.com/~john/ticket/3082/test/?idth (width tests)
The patch:
http://dev.jquery.com/attachment/ticket/3082/3082.patch
Landing:
http://dev.jquery.com/changeset/6195
The result is even faster than what Mike originally proposed (as you
can see from the perf test suite). I'm seeing 10-20% over Mike's on
height/width, 20-30% on innerHeight/Width, 100% on outerHeight/Width,
and 10-40% on outerHeight/Width(true).
Thanks a ton, Mike, for this set of tweaks - it's a huge win!
--John

sound of one keyboard clapping

Very nice... all so quick too! :)
--
Brandon Aaron

good stuff, thanks for taking it the rest of the way.
-mike

http://dev.jquery.com/ticket/4276
Clean-up some unused, orphaned code.