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] PTO during Sep 22 ~ 30
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-28 08:15 PDT by Thinker Li [:sinker] PTO during Sep 22 ~ 30
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] PTO during Sep 22 ~ 30
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] PTO during Sep 22 ~ 30
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] PTO during Sep 22 ~ 30
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] PTO during Sep 22 ~ 30
tlee: review+
Details | Diff | Splinter Review

Description Thinker Li [:sinker] PTO during Sep 22 ~ 30 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] PTO during Sep 22 ~ 30 2012-06-28 08:17:42 PDT
Created attachment 637520 [details] [diff] [review]
WIP
Comment 2 Thinker Li [:sinker] PTO during Sep 22 ~ 30 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] PTO during Sep 22 ~ 30 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] PTO during Sep 22 ~ 30 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] PTO during Sep 22 ~ 30 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.