Navigation Menu

Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Optimize CodeHintList DOM build #3776

Merged
merged 2 commits into from May 10, 2013
Merged

Conversation

jasonsanjose
Copy link
Member

  • Batch up non-jQuery hints into a single template rendering. In my initial Timeline test, showing the full list of HTML hints loading (HTML parsing) elapsed time went from 105ms to 3ms
  • For jQuery-based hints, still batch up the outer li template generation, then add append jQuery content afterwards. When showing the full list of jQuery hints, elapsed time went from 51ms to 1ms
  • For both, remove the use of the function closure and use a data- attribute to store hint data for handleSelect
  • For both, use a event delegate for the parent ul instead of individual event handlers for each li

@ghost ghost assigned RaymondLim May 10, 2013
@@ -308,7 +328,7 @@ define(function (require, exports, module) {
} else if (this.selectedIndex !== -1 &&
(keyCode === KeyEvent.DOM_VK_RETURN || keyCode === KeyEvent.DOM_VK_TAB)) {
// Trigger a click handler to commmit the selected item
$(this.$hintMenu.find("li")[this.selectedIndex]).triggerHandler("click");
$(this.$hintMenu.find("li")[this.selectedIndex]).trigger("click");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reposting here as I was incorrectly commenting on the commit.

I'm not clear why this is more efficient than triggerHandler. I know that triggerHandlers do not bubble up the DOM hierarchy. So how can a trigger call be faster if it does propagation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think we're talking about different issues here.

On line 28 we create a single delegate event handler (which relies on event bubbling). Previously, we would install event handlers on each li. So, instead of taking the performance hit when building the DOM, we now have a negligible hit waiting for the event to bubble. So it's faster to load the code hints, but slightly slower to execute the code hint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for excellent explanation. Now I'm clear where you're improving the performance.

@RaymondLim
Copy link
Contributor

In my testing the new code seems to pop up code hints faster when I tried with js hinting. And selecting hint with Tab or Enter key doesn't seem to be slower even though we're using "trigger" call.

So looks good. Merging.

RaymondLim added a commit that referenced this pull request May 10, 2013
@RaymondLim RaymondLim merged commit 2071f72 into master May 10, 2013
@RaymondLim RaymondLim deleted the jasonsanjose/perf-code-hints branch May 10, 2013 22:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants