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

Fixes #453 : Keep strings in cache instead of jquery objects #465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixes #453 : Keep strings in cache instead of jquery objects #465

wants to merge 1 commit into from

Conversation

klevron
Copy link

@klevron klevron commented Dec 6, 2014

No description provided.

@klevron
Copy link
Author

klevron commented Dec 6, 2014

#453

@mislav
Copy link
Collaborator

mislav commented Dec 7, 2014

This might slow down navigation due to the extra overhead of HTML serialization and parsing, and might have other adverse side-effects as well. If you want to make a significant change like this happen in a library, you should try making a stronger case than just a pull request with no description.

Like, you might explore:

  1. Does this actually use less cache memory (something that you hinted at in the issue discussion)
  2. How does the performance of this compare to the previous approach on large pages?
  3. What could be some less invasive ways to solve the VIDEO problem loading/playing in background?

@klevron
Copy link
Author

klevron commented Dec 7, 2014

Hello

Sorry, didn't add description here since discussion is in the issue...

About memory, don't have time to make a bench, but it's obvious, nodes will take more memory than simple strings.

About perfs : http://jsperf.com/jquery-clone-vs-html-construction
Don't have time now, but it should be interesting to complete this bench with node inserts.

And about video bug, I tried a lot of things, it is the better and simplest way I found to solve it.
So, is there any reason to keep jquery objects in cache ?

One caveat in using html() :

This method uses the browser's innerHTML property. Some browsers may not return HTML that exactly replicates the HTML source in an original document

@mislav
Copy link
Collaborator

mislav commented Dec 8, 2014

Some browsers may not return HTML that exactly replicates the HTML source in an original document

Yeah that's what I'm worried about. We can explore this direction, and I can definitely believe that HTML strings can make up less memory than storing nodes, but we'll need to perform some testing on real-world-looking pages with dozens or even hundreds of navigations between reloads and analyze how it affects performance stats.

@klevron
Copy link
Author

klevron commented Dec 8, 2014

I'll try to build a bench in the week.
We could also add an option to chose cache type, what do you think ?

@klevron
Copy link
Author

klevron commented Dec 8, 2014

Little bench with more tags : http://jsperf.com/clone-vs-html-bench
The difference is not so visible in chrome.

@mislav
Copy link
Collaborator

mislav commented Dec 9, 2014

On Mon, Dec 8, 2014 at 8:29 AM, Kevin LEVRON [email protected]
wrote:

The difference is not so visible in chrome.

Thanks for making that bench. Interesting that the html() approach is
just as fast or faster than cloning nodes.

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.

2 participants