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]
:
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 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 Thinker Li [:sinker] 2012-06-28 08:17:42 PDT
Created attachment 637520 [details] [diff] [review]
WIP
Comment 2 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 Honza Bambas (:mayhemer) 2012-06-28 11:06:07 PDT
Thinker, can you confirm that access to sqlite is thread-safe?
Comment 4 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 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 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 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 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 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 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 Andreas Gal :gal 2012-07-10 07:18:02 PDT
Thinker, lets land this.
Comment 12 Kan-Ru Chen [:kanru] (UTC+8) 2012-07-10 23:17:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f9bf790012b
Comment 13 Thinker Li [:sinker] 2012-07-10 23:19:26 PDT
try https://tbpl.mozilla.org/?tree=Try&rev=e6589935be49
Comment 14 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.