[CLOSED] Consider 2,000 lines to be a large file for hints.

[core-ai-bot]

Issue by dangoor
Wednesday Oct 02, 2013 at 20:02 GMT
Originally opened as adobe/brackets#5400

---

This is a fix for #4922... only part of Document.js was being processed
because it was considered a "large file" at 250 lines. 250 lines seems
a bit too conservative. We'll want to see if there are complaints about
memory usage and I think we'll also want to experiment a bit with how
we manage Tern.

But, it seems to me that a file the size of Document.js should not be
getting truncated hints.

---

dangoor included the following code: https://github.com/adobe/brackets/pull/5400/commits

[core-ai-bot]

Comment by dangoor
Thursday Oct 03, 2013 at 12:54 GMT

---

Oops. Simple change that fixes the problem, but I forgot to run the tests. There are some failures to fix.

[core-ai-bot]

Comment by dangoor
Thursday Oct 03, 2013 at 14:33 GMT

---

The unit tests were failing because one of the sample files had been padded to be above 250 lines (making it a "long file" by the old definition, thus triggering the partial update logic).

This seemingly minor change actually worries me now. I padded the file to be 2,000 lines long, triggering the new long file limit. The tests ran a lot slower. Additionally, I closed Brackets, relaunched and ran the code hinting tests. brackets-helper grew to nearly 500MB in memory usage and did not drop. I stashed my changes and went back to the 250 line version of the file. brackets-helper only grew to 215MB. Note that the only difference between the old file and the new is 1750 more lines of comment (no additional JS that Tern would be working with).

I don't know if this memory leak is due to the way the tests are written or if it's due to the way the code hinting extension is written.

[core-ai-bot]

Comment by dangoor
Thursday Oct 03, 2013 at 15:52 GMT

---

Some testing and profiling later... it appears most likely that the issue I'm seeing is with the tests and not with the original change. Most of the CPU and memory in the running of the tests seems to be going into the test editor instances.

I did a test where I opened a fresh Brackets up with a project that contained only Document.js. With this change, memory usage was about 3MB more than before this change, which does not seem completely out of line given the extra data we're tracking by looking the whole file rather than just 250 lines.

I'm going to try to make the tests faster and less memory hungry.

[core-ai-bot]

Comment by dangoor
Thursday Oct 24, 2013 at 16:12 GMT

---

I left the test file (file1.js) at the same size that it was previously. When doing that, there were a small number of tests that failed purely for timing reasons (run individually, they work fine... run in the suite, they do not). These tests can be fairly unpredictable because the code is designed to make sure that results are returned quickly.

I think there are some things we can do that will make the behavior faster and more predictable.

In the meantime, I think it's safe to set this limit to 2000 lines. We might consider waiting until the beginning of next sprint, though.

[core-ai-bot]

Comment by jasonsanjose
Friday Nov 01, 2013 at 18:25 GMT

---

Filed #5811 to capture re-enabling the tests. Merging.