Increase input history limit

RESOLVED FIXED in Firefox 64

Status

enhancement
P2
normal
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: nchevobbe, Assigned: dev, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 64

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Currently, we only store the last 50 entries [1].
Since we added reverse search not long ago, I feel like 50 might be a bit small.
We could increase this to an higher number, which would be determined by the impact on performances ?

For what it's worth, Chrome does seem to keep the latest 300 entries.

[1] https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/devtools/client/preferences/devtools-client.js#265-266
Mentor: nchevobbe
Keywords: good-first-bug
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #0)
> Currently, we only store the last 50 entries [1].
> Since we added reverse search not long ago, I feel like 50 might be a bit
> small.
> We could increase this to an higher number, which would be determined by the
> impact on performances ?
> 
> For what it's worth, Chrome does seem to keep the latest 300 entries.
> 
> [1]
> https://searchfox.org/mozilla-central/rev/
> bdc89dfd7869e418d788b28eb60ab8d94e708a15/devtools/client/preferences/
> devtools-client.js#265-266

Hey I'd love to pick up this feature! It'd be my first. So do you think 300 would be a good number to go by then?
Hello dev ! Great to hear you want to help.
So here, we can try 300, and then we'll profile if it has a too important impact on opening the console/loading the history.

If you haven't already, I suggest you to read https://docs.firefox-dev.tools/getting-started/ to get all the information on how to setup the work environment. This should refer to "artifact builds" at some point, which is something I recommend to use (it downloads the binary instead of compiling them locally, which is way faster on a decent connection).

Then, once you have everything set-up, and a patch ready for review, we can talk here on how to submit a patch, and the way we can check if it impacts the performances.

Finally, you can join https://devtools-html-slack.herokuapp.com/ if you have issues or simply want to chat 😀
Assignee: nobody → dev
Status: NEW → ASSIGNED
Great I'll get on that. Thank you so much for the help!
Comment on attachment 9010298 [details]
Bug 1491768  - Increase input history limit; r=nchevobbe.

Nicolas Chevobbe [:nchevobbe] has approved the revision.
Attachment #9010298 - Flags: review+
So, TRY is over https://treeherder.mozilla.org/#/jobs?repo=try&revision=396a595b397b13509b0d08e177aa9529f0f2caca 
You can see that everything is okay, except one test. This is not something on the devtools, and might be what we call orange/intermittent, i.e. a test that sometimes fail.

I also did some profiling, and with an history of 300 inputs, it takes about 5 to 20ms to load the entries, which seems totally fine to me (and we're not blocking anything anyway).

So let's land this patch !
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/639573673421
Increase input history limit; r=nchevobbe.
https://hg.mozilla.org/mozilla-central/rev/639573673421
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Great news dev, your code will be in tomorrow's Firefox Nightly ! :)
Thanks a lot for taking this 👍
Thanks so much for your help! Look forward to contributing further.
You need to log in before you can comment on or make changes to this bug.