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.

Sign up for GitHub

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

Jump to bottom

removeView performance #447

Closed
johan-ohrn opened this issue May 17, 2020 · 2 comments
Closed

removeView performance #447

johan-ohrn opened this issue May 17, 2020 · 2 comments
Labels
Optimization Resolved

Comments

@johan-ohrn
Copy link

(I found this issue in the WIP cache version of jquery.views.js but I'm pretty sure it's in the current version as well)

at line 2920 in jquery.views.js:

try {
  prevNode.parentNode.removeChild(prevNode); // (prevNode.parentNode is parentElem, except if jQuery Mobile or similar has inserted an intermediate wrapper
  nextNode.parentNode.removeChild(nextNode);
} catch (e) { }

Executing this code with one of my templates take ~140ms

//try {
  prevNode && prevNode.parentNode.removeChild(prevNode); // (prevNode.parentNode is parentElem, except if jQuery Mobile or similar has inserted an intermediate wrapper
  nextNode && nextNode.parentNode.removeChild(nextNode);
//} catch (e) { }

Executing this code with the same templates take ~40ms

prevNode and? nextNode is undefined at times.
@BorisMoore
Copy link
Owner

Yes, you are right about the perf cost, and indeed I think we can simply replace that code by:

if (!viewToRemove._elCnt && prevNode) {
	prevNode.parentNode.removeChild(prevNode); // ...
	nextNode.parentNode.removeChild(nextNode);
}

@BorisMoore BorisMoore mentioned this issue May 20, 2020
Closed
BorisMoore added a commit to BorisMoore/jsviews.com that referenced this issue Jul 12, 2020
@BorisMoore
Commit v1.0.7
e4d6ce9 
Feature improvements for JsRender on Node.js:

  (See issue BorisMoore/jsrender#357)

- File based templates for JsRender on Node.js now allow
  absolute paths

  See the updated documentation topic:
  https://www.jsviews.com/#node/filetmpls@htmlfile

- The renderFile() method for JsRender on Node.js
  now allows passing of context and helpers as well
  as data

  See the new documentation topic:
  https://www.jsviews.com/#node/filetmpls@renderfilehlelpers

Performance optimization changes:

- BorisMoore/jsviews#448
  $.observe/$.unobserve performance degradation over time

- BorisMoore/jsviews#447
  removeView performance

Several bug fixes, including:

- BorisMoore/jsviews#446
  Observable change error, when loading jquery.observable.js before
  jsrender.js, and using contextual parameters and sorted {{for}}

- BorisMoore/jsviews#440
  linked tag property changed handler called when it shouldn't

- BorisMoore/jsviews#445
  jsrender.min.js.map truncated

Additional small corrections or improvements to documentation
and some additional unit tests...
BorisMoore added a commit that referenced this issue Jul 12, 2020
@BorisMoore
v1.0.7 release
3b3f669 
Feature improvements for JsRender on Node.js:

  (See issue BorisMoore/jsrender#357)

- File based templates for JsRender on Node.js now allow
  absolute paths

  See the updated documentation topic:
  https://www.jsviews.com/#node/filetmpls@htmlfile

- The renderFile() method for JsRender on Node.js
  now allows passing of context and helpers as well
  as data

  See the new documentation topic:
  https://www.jsviews.com/#node/filetmpls@renderfilehlelpers

Performance optimization changes:

- #448
  $.observe/$.unobserve performance degradation over time

- #447
  removeView performance

Several bug fixes, including:

- #446
  Observable change error, when loading jquery.observable.js before
  jsrender.js, and using contextual parameters and sorted {{for}}

- #440
  linked tag property changed handler called when it shouldn't

- #445
  jsrender.min.js.map truncated

Additional small corrections or improvements to documentation
and some additional unit tests...
@BorisMoore
Copy link
Owner

This has been fixed in new release v1.0.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Optimization Resolved
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@BorisMoore @johan-ohrn