Closed Bug 1491768 Opened 2 years ago Closed 2 years ago
Increase input history limit
46 bytes, text/x-phabricator-request
|Details | Review|
Currently, we only store the last 50 entries . 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.  https://searchfox.org/mozilla-central/rev/bdc89dfd7869e418d788b28eb60ab8d94e708a15/devtools/client/preferences/devtools-client.js#265-266
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #0) > Currently, we only store the last 50 entries . > 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. > >  > 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 firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/639573673421 Increase input history limit; r=nchevobbe.
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.