Closed Bug 966469 Opened 6 years ago Closed 5 years ago

Slow queries in netpredictions.sqlite

Categories

(Core :: Networking, defect, major)

29 Branch
defect
Not set
major

Tracking

()

RESOLVED DUPLICATE of bug 1009122

People

(Reporter: rnewman, Unassigned)

Details

(Keywords: perf, regression)

Was poking around in about:telemetry on my phone, saw these.

Hits Avg. Time (ms) Statement
8 126 COMMIT TRANSACTION /* netpredictions.sqlite */
1 138 BEGIN DEFERRED /* netpredictions.sqlite */
9 118 PRAGMA page_count; /* netpredictions.sqlite */
1 103 PRAGMA page_size; /* netpredictions.sqlite */
Keywords: perf
On Windows 7, checking "Slow SQL Statements on Helper Threads" on about:telemetry I'm seeing, right now:

1 	109 	COMMIT TRANSACTION /* netpredictions.sqlite */ 
1 	2192 	VACUUM; /* netpredictions.sqlite */ 
1 	5198 	DELETE FROM moz_subresources WHERE id IN (SELECT id FROM moz_subresources ORDER BY last_hit ASC LIMIT :limit); /* netpredictions.sqlite */ 
1 	1109 	SELECT COUNT(id) FROM moz_subresources /* netpredictions.sqlite */ 
1 	21966 	DELETE FROM moz_pages WHERE id NOT IN (SELECT DISTINCT(pid) FROM moz_subresources); /* netpredictions.sqlite */

Nightly is open for a few hours.
Ouch. Those are some nasty numbers for a desktop.
Blocks: 947745
Severity: normal → major
Flags: firefox-backlog?
Flags: firefox-backlog? → firefox-backlog-
We are seeing a rise in reports of Firefox.exe not closing quickly on exit, and some contributors on the support forum feels this may be the cause. One specific case, https://support.mozilla.org/en-US/questions/998449


Can evaluate for a 29.0.1? Thanks!
Keywords: qawanted
Flags: needinfo?(hurley)
netpredictions/Seer is a new feature in 29, not sure how much it even brings for the user yet.
We had a few crashes with that database that got fixed in the 29 cycle, if we still have that bad SQL slowness there, I wonder if it actually was ship-ready, though.
Does that impact also 30 & 31?
For more context, see also Bug 947745 (disk size regression), Bug 945779 (CPU regression), Bug 961209 and Bug 960834 (storing too much data).

Nick Hurley should have the scoop on whether 30 or 31 are better re this bug.
It would be nice to see some synthetic benchmark to compare performance with and without this feature.
(In reply to Virtual_ManPL [:Virtual] from comment #8)
> It would be nice to see some synthetic benchmark to compare performance with
> and without this feature.

We have one, see bug 881804 comment 103 (specifically the part about a 10-25% improvement in SpeedIndex on repeated page loads).
Flags: needinfo?(hurley)
The bug to disable this feature on all branches is bug 1005958
(In reply to Sylvestre Ledru [:sylvestre] from comment #6)
> Does that impact also 30 & 31?

Yes.

(In reply to Nicholas Hurley [:hurley] from comment #9)
> (In reply to Virtual_ManPL [:Virtual] from comment #8)
> > It would be nice to see some synthetic benchmark to compare performance with
> > and without this feature.
> 
> We have one, see bug 881804 comment 103 (specifically the part about a
> 10-25% improvement in SpeedIndex on repeated page loads).

Thank you. Missed that one.
We've taken bug 1005958 for FF29.0.1, tracking this bug for the better fix in later versions.
Here is data from Telemetry on Firefox/Win 7 v30 beta channel soon after a restart of Firefox.

When I have Seer enabled via network.seer.enabled I often have Firefox and other applications reporting "Not Responding" and slow response times.

Slow SQL Statements on Main Thread Hits 	Avg. Time (ms) 	Statement
2 	942 	COMMIT TRANSACTION /* places.sqlite */
Slow SQL Statements on Helper Threads Hits 	Avg. Time (ms) 	Statement
1 	81826 	DELETE FROM moz_pages; /* netpredictions.sqlite */
4 	202 	UPDATE moz_hosts SET permission = ?2, expireType= ?3, expireTime = ?4 WHERE id = ?1 /* permissions.sqlite */
1 	146 	DELETE FROM moz_subresources WHERE id IN (SELECT id FROM moz_subresources ORDER BY last_hit ASC LIMIT :limit); /* netpredictions.sqlite */
1 	123 	/* c:/builds/moz2_slave/rel-m-beta-w32_bld-00000000000/build/storage/src/mozStorageConnection.cpp */ PRAGMA cache_size = -2048 /* netpredictions.sqlite */
3 	348 	COMMIT TRANSACTION /* places.sqlite */
1 	133 	SELECT DISTINCT scope FROM webappsstore2 /* webappsstore.sqlite */
Depends on: 1007615
Nick - what's the status of this bug? Is the main work being done elsewhere for the 'proper' fix here?  Do we need to look at doing the same disabling in FF30 at this stage? We're two weeks away from RC.
Flags: needinfo?(hurley)
Lukas - this bug will (eventually) be resolved by the backend rewrite (bug 1009122). Until then, the seer has been disabled on all branches, so we don't need to worry about this for any upcoming releases.
Flags: needinfo?(hurley)
Thanks Nick, no longer need to track in that case.
It looks like the activity on this is happening in bug 1009122 so I'm removing the qawanted tag. 
Nicholas, is it reasonable to WONTFIX this bug based on Comment 15?
Flags: needinfo?(hurley)
Keywords: qawanted
(In reply to Liz Henry :lizzard from comment #17)
> Nicholas, is it reasonable to WONTFIX this bug based on Comment 15?

Yep. There are actually quite a few bugs that will have to be WONTFIXed, but I'll track the rest down once I finish bug 1009122 :)
Flags: needinfo?(hurley)
more like marked as WORKSFORME or DUPLICATE, as they will be "FIXED" by rewriting Seer ;d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1009122
You need to log in before you can comment on or make changes to this bug.