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

Rewrite of dependencies.js #9472

Closed
wants to merge 4 commits into from
Closed

Rewrite of dependencies.js #9472

wants to merge 4 commits into from

Conversation

TuurDutoit
Copy link
Contributor

The original code wasn't very optimized (8+ document.write calls!), so I rewrote it to be a little more performant.
Now I get an execution time of about 0.05ms, compared to 0.5ms before.

I just dived into Brackets' codebase, with the intention of optimizing some code (Brackets still feels a little slow), and this was an easy first pick.

Less code + greater performance = WIN!
About 0.5ms performance gain on my machine, primarly by touching the DOM just once.
@TuurDutoit
Copy link
Contributor Author

Why is that setTimeout() there, actually? It looks like an ugly hack...

@TuurDutoit
Copy link
Contributor Author

I read that, normally, I shouldn't use a for...in-loop, but rather _.forEach. But I wonder if _ is already available at that moment. The original author used a for...in.

@le717
Copy link
Contributor

le717 commented Oct 7, 2014

Can you merge this with master so the Travis build can run? :)

@TuurDutoit
Copy link
Contributor Author

My master, I suppose?

@le717
Copy link
Contributor

le717 commented Oct 7, 2014

No, merge with the latest changes here in this, the main repo. Directions are on the wiki.

@TuurDutoit
Copy link
Contributor Author

I'll do that tomorrow, I'm on mobile now. Is that ok?

@le717
Copy link
Contributor

le717 commented Oct 7, 2014

Whenever you can is fine. I'm just giving you a heads up for when a dev can look at this. 😉

@TuurDutoit
Copy link
Contributor Author

My repo is getting too messy. I'm forking again, and doing it right, this time (I hope)

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.

2 participants