Closed Bug 395735 Opened 17 years ago Closed 17 years ago

Instrument the location bar auto-complete to report accuracy

Categories

(Firefox :: Address Bar, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta4

People

(Reporter: faaborg, Assigned: Mardak)

References

Details

Attachments

(1 file, 5 obsolete files)

As we tweak the algorithm for auto completion in the location bar, we need actual data to understand which techniques work better than others. For each time the user enters text in the location bar, what if we recorded which result they selected, and then aggregated and reported this information back? During the beta testing phase of Firefox 3 we could then change the algorithm in the nightly builds, and get data on if we made things better or worse.
Note: I am only proposing we have this instrumentation in for beta testing, we would remove it before actually shipping Firefox 3. And by "which result they selected" I mean in order, not the actual result (like they selected the third result).
Is this still wanted for beta; Beta 2 is coming soon :)
How much different is this from the adaptive feedback logging for bug 395739? It /could/ make use of 'how far down' the selected item is, but right now it just bumps up a counter pushing the result higher up for next time.
>How much different is this from the adaptive feedback logging for bug 395739? adaptive feedback essentially invalidates the overall goal of this bug, which was for us to find ways to tweak our algorithm to produce better results. If the results are adapting themselves based on user behavior, than that is likely to be the optimal solution.
Attached patch v1 (obsolete) (deleted) — Splinter Review
This is the feedback part of the adaptive learning url bar autocomplete. Adds nsIAutoCompleteFeedback and updates AutoCompleteController to call the feedback mechanism, but no actual implementation of the feedback (other than the testcase).
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #290961 - Flags: review?(sspitzer)
Bug 395739 will implement nsIAutoCompleteFeedback for nsNavHistory.
Blocks: awesomebar
Just wanted to point out that we implemented this for Fx2... not sure how much of it is still relevant... but it may serve as a model for what we are trying to do. http://mxr.mozilla.org/seamonkey/source/extensions/metrics/src/nsAutoCompleteCollector.cpp Updating this code would allow us to integrate this into the Spectator project.
Attached patch v1.1 (obsolete) (deleted) — Splinter Review
Fix up the testcase to correctly break references when it's done.
Attachment #290961 - Attachment is obsolete: true
Attachment #298855 - Flags: review?(dietrich)
Attachment #290961 - Flags: review?(moco)
Comment on attachment 298855 [details] [diff] [review] v1.1 I'll probably switch the instrumentation to use the observer service that the AutoCompleteController is already notifying.
Attachment #298855 - Flags: review?(dietrich)
Attached patch v2 (obsolete) (deleted) — Splinter Review
Use the observer service instead of creating a new interface.
Attachment #298855 - Attachment is obsolete: true
Attachment #301683 - Flags: review?
Attached patch v3 (obsolete) (deleted) — Splinter Review
Consolidate all the moz_inputhistory feedback stuff here instead of having the dummy AutoCompleteFeedback do nothing. Testcase is in bug 395739.
Attachment #301683 - Attachment is obsolete: true
Attachment #302076 - Flags: review?
Attachment #301683 - Flags: review?
Comment on attachment 302076 [details] [diff] [review] v3 Use the existing observer and store the data in moz_inputhistory.
Attachment #302076 - Flags: review? → review?(dietrich)
Attached patch v3.1 (obsolete) (deleted) — Splinter Review
Unbitrotting assuming that bug 401660 will land first which will help blocking bug 405320. Whereas this bug is in the line to help blocking bug 411293.
Attachment #302076 - Attachment is obsolete: true
Attachment #304078 - Flags: review?(dietrich)
Attachment #302076 - Flags: review?(dietrich)
Dietrich: this bug is blocking a review and landing of adaptive learning, which would make the awesome bar so much more awesome, any chance you will have cycles to get the review done? Requesting blocking in the event that bug 393739 also receives blocking.
Flags: blocking-firefox3?
Comment on attachment 304078 [details] [diff] [review] v3.1 >+ // moz_inputhistory >+ rv = mDBConn->TableExists(NS_LITERAL_CSTRING("moz_inputhistory"), &tableExists); >+ NS_ENSURE_SUCCESS(rv, rv); >+ if (!tableExists) { >+ rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING("CREATE TABLE moz_inputhistory (" >+ "place_id INTEGER NOT NULL, " >+ "input LONGVARCHAR NOT NULL, " >+ "use_count INTEGER, " >+ "PRIMARY KEY (place_id, input))")); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( >+ "CREATE INDEX moz_inputhistory_placeindex ON moz_inputhistory (place_id)")); > NS_ENSURE_SUCCESS(rv, rv); > } this index is redundant, i think. the index created by your primary key should be able to be utilized here. you should do EXPLAIN QUERY PLAN to confirm this though (and if i'm wrong, to confirm that your new index is in fact being used, if you haven't done this already). >+ sql = NS_LITERAL_CSTRING( >+ // Leverage the PRIMARY KEY (place_id, input) to insert/update entries >+ "INSERT OR REPLACE INTO moz_inputhistory " >+ // use_count will asymptotically approach the max of 10 >+ "SELECT h.id, IFNULL(i.input, ?2), IFNULL(i.use_count, 0) * .9 + 1 " >+ "FROM moz_places h " >+ "LEFT OUTER JOIN moz_inputhistory i ON i.place_id = h.id AND i.input = ?2 " >+ "WHERE h.url = ?1"); >+ rv = mDBConn->CreateStatement(sql, getter_AddRefs(mDBFeedbackIncrease)); > NS_ENSURE_SUCCESS(rv, rv); nit: why ?2 before ?1 r=me with these fixed.
Attachment #304078 - Flags: review?(dietrich) → review+
Attached patch v3.2 (deleted) — Splinter Review
(In reply to comment #15) > >+ "CREATE INDEX moz_inputhistory_placeindex ON moz_inputhistory (place_id)")); > this index is redundant. you should do EXPLAIN QUERY PLAN to confirm this Neato. For the INSERT OR REPLACE query to update use_counts, it's using moz_places_url_uniqueindex and sqlite_autoindex_moz_inputhistory as expected. But the query to select adaptive results for bug 395739 uses the primary key of moz_places on moz_places.id because the query is a moz_inputhistory JOIN moz_places not a LEFT OUTER JOIN. So I've removed the index because it's redundant in both cases. > >+ "INSERT OR REPLACE INTO moz_inputhistory " > >+ "SELECT h.id, IFNULL(i.input, ?2), IFNULL(i.use_count, 0) * .9 + 1 " > >+ "FROM moz_places h " > >+ "LEFT OUTER JOIN moz_inputhistory i ON i.place_id = h.id AND i.input = ?2 " > >+ "WHERE h.url = ?1"); > nit: why ?2 before ?1 Well, I think I wrote the code to use the parameters before the query, but I reordered it.
Attachment #304078 - Attachment is obsolete: true
Attachment #305377 - Flags: review+
Attachment #305377 - Flags: approval1.9?
Whiteboard: [needed for bug 395739]
Comment on attachment 305377 [details] [diff] [review] v3.2 a1.9+=damons
Attachment #305377 - Flags: approval1.9? → approval1.9+
Testcase will land with bug 395739. Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.260; previous revision: 1.259 done Checking in toolkit/components/places/src/nsNavHistory.h; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v <-- nsNavHistory.h new revision: 1.142; previous revision: 1.141 done Checking in toolkit/components/places/src/nsNavHistoryAutoComplete.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp,v <-- nsNavHistoryAutoComplete.cpp new revision: 1.44; previous revision: 1.43 done Checking in toolkit/components/places/src/nsNavHistoryExpire.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v <-- nsNavHistoryExpire.cpp new revision: 1.42; previous revision: 1.41 done Checking in toolkit/components/places/src/nsNavHistoryExpire.h; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.h,v <-- nsNavHistoryExpire.h new revision: 1.10; previous revision: 1.9 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta4
Flags: blocking-firefox3? → blocking-firefox3+
Depends on: 419656
Whiteboard: [needed for bug 395739]
Depends on: 453821
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: