Last Comment Bug 769291 - Offload evicting offline caches to Cache IO thread.
: Offload evicting offline caches to Cache IO thread.
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: Cache (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Thinker Li [:sinker]
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-28 08:15 PDT by Thinker Li [:sinker]
Modified: 2012-07-11 09:31 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (3.80 KB, patch)
2012-06-28 08:17 PDT, Thinker Li [:sinker]
no flags Details | Diff | Splinter Review
Move nsOfflineCacheDevice::Discard() to cache IO thread (1.83 KB, patch)
2012-06-28 10:19 PDT, Thinker Li [:sinker]
no flags Details | Diff | Splinter Review
Move nsOfflineCacheDevice::Discard() to cache IO thread, v2 (2.24 KB, patch)
2012-07-03 01:22 PDT, Thinker Li [:sinker]
honzab.moz: review+
Details | Diff | Splinter Review
Move nsOfflineCacheDevice::Discard() to cache IO thread, v3 (2.24 KB, patch)
2012-07-09 20:56 PDT, Thinker Li [:sinker]
tlee: review+
Details | Diff | Splinter Review

Description User image Thinker Li [:sinker] 2012-06-28 08:15:41 PDT
OfflineCacheDevice remove entry data from database for evicting at main thread.  Remove these data tasks to cache IO thread can improve the performance.
Comment 1 User image Thinker Li [:sinker] 2012-06-28 08:17:42 PDT
Created attachment 637520 [details] [diff] [review]
WIP
Comment 2 User image Thinker Li [:sinker] 2012-06-28 10:19:46 PDT
Created attachment 637572 [details] [diff] [review]
Move nsOfflineCacheDevice::Discard() to cache IO thread
Comment 3 User image Honza Bambas (:mayhemer) 2012-06-28 11:06:07 PDT
Thinker, can you confirm that access to sqlite is thread-safe?
Comment 4 User image Andreas Gal :gal 2012-06-28 11:27:39 PDT
We already use SQLite from two threads here. This patch reduces that which is why it's faster (we lock the main thread without this patch).
Comment 5 User image Honza Bambas (:mayhemer) 2012-06-28 11:33:54 PDT
Doing something doesn't mean it is safe.  Could anyone here confirm that accessing the connection is really thread safe?  From my experience it is not, but things might change since that time.
Comment 6 User image Andreas Gal :gal 2012-06-28 12:47:57 PDT
The connection is thread safe. We use it currently from multiple threads. With this patch we only use it from one thread.
Comment 7 User image Honza Bambas (:mayhemer) 2012-06-28 14:18:11 PDT
(In reply to Andreas Gal :gal from comment #6)
> The connection is thread safe. We use it currently from multiple threads.
> With this patch we only use it from one thread.

Thanks.
Comment 8 User image Thinker Li [:sinker] 2012-07-03 01:22:08 PDT
Created attachment 638617 [details] [diff] [review]
Move nsOfflineCacheDevice::Discard() to cache IO thread, v2
Comment 9 User image Honza Bambas (:mayhemer) 2012-07-05 14:14:31 PDT
Comment on attachment 638617 [details] [diff] [review]
Move nsOfflineCacheDevice::Discard() to cache IO thread, v2

Review of attachment 638617 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab.  Let's get this in.  This is a good patch.
Comment 10 User image Thinker Li [:sinker] 2012-07-09 20:56:18 PDT
Created attachment 640492 [details] [diff] [review]
Move nsOfflineCacheDevice::Discard() to cache IO thread, v3

r=honzab
Comment 11 User image Andreas Gal :gal 2012-07-10 07:18:02 PDT
Thinker, lets land this.
Comment 12 User image Kan-Ru Chen [:kanru] (UTC+8) 2012-07-10 23:17:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f9bf790012b
Comment 13 User image Thinker Li [:sinker] 2012-07-10 23:19:26 PDT
try https://tbpl.mozilla.org/?tree=Try&rev=e6589935be49
Comment 14 User image Ed Morley [:emorley] 2012-07-11 09:31:02 PDT
https://hg.mozilla.org/mozilla-central/rev/5f9bf790012b

Note You need to log in before you can comment on or make changes to this bug.