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

[DOM]: Don't cache throw away elements. #1773

Merged
merged 3 commits into from Apr 24, 2014
Merged

Conversation

ezequiel
Copy link
Contributor

@ezequiel ezequiel commented Apr 9, 2014

Whenever you use Y.Node.create or the like, it'll create a "dummy" element under the hood to do its work on. This dummy element is cached in _fragClones, while it's children are intermittently removed later on. Since this dummy element is never cleared from the cache, a small (potentially) memory leak is caused.

I suppose the idea behind caching dummy elements in _fragClones was to avoid the constant creation of these elements when using Y.Node.create.

If you look closely, cloneNode(false) is called if the particular dummy element is found in the cache. Shallow cloning an element is hardly faster than simply creating it from scratch. The difference in speed is in the noise. See: http://jsperf.com/clone-vs-create-part-deux

I rather us avoid caching the dummy element to avoid the memory leak all together since speed is not a factor.

@yahoocla
Copy link

yahoocla commented Apr 9, 2014

CLA is valid!

@rgrove
Copy link
Contributor

rgrove commented Apr 10, 2014

Nice! 👍

@tripp
Copy link
Contributor

tripp commented Apr 10, 2014

Yeah. Good stuff.

@ezequiel ezequiel merged commit c003658 into yui:dev-master Apr 24, 2014
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.

None yet

4 participants