Closed
Bug 84224
Opened 23 years ago
Closed 23 years ago
Trying to open cookperm.txt for every image load
Categories
(Core :: Graphics: Image Blocking, defect, P2)
Core
Graphics: Image Blocking
Tracking
()
RESOLVED
FIXED
mozilla0.9.2
People
(Reporter: sfraser_bugs, Assigned: morse)
References
Details
(Keywords: perf)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
My windows build is trying to open cookperm.txt for every image load in the
chrome, which seems suboptimal. Stack:
PERMISSION_Read() line 432
IMAGE_CheckForPermission(const char * 0x04237f30, const char * 0x04235f30, int *
0x0012e2c8) line 171 + 5 bytes
nsImgManager::ShouldLoad(nsImgManager * const 0x04222364, int 2, nsIURI *
0x04236110, nsISupports * 0x03d13794, nsIDOMWindow * 0x01318cd4, int *
0x0012e2c8) line 96 + 29 bytes
nsContentPolicy::CheckPolicy(nsContentPolicy * const 0x04222650, int 0, int 2,
nsIURI * 0x04236110, nsISupports * 0x03d13794, nsIDOMWindow * 0x01318cd4, int *
0x0012e2c8) line 139 + 43 bytes
nsContentPolicy::ShouldLoad(nsContentPolicy * const 0x04222650, int 2, nsIURI *
0x04236110, nsISupports * 0x03d13794, nsIDOMWindow * 0x01318cd4, int *
0x0012e2c8) line 166
NS_CheckContentLoadPolicy(int 2, nsIURI * 0x04236110, nsISupports * 0x03d13794,
nsIDOMWindow * 0x01318cd4, int * 0x0012e2c8) line 56 + 112 bytes
nsPresContext::StartLoadImage(nsPresContext * const 0x03143bf0, const nsString &
{...}, const unsigned int * 0x00000000, const nsSize * 0x00000000, nsIFrame *
0x02cf633c, unsigned int (nsIPresContext *, nsIFrameImageLoader *, nsIFrame *,
void *, unsigned int)* 0x00000000, void * 0x00000000, void * 0x02cf633c,
nsIFrameImageLoader * * 0x0012e3a0) line 1270 + 38 bytes
nsCSSRendering::PaintBackground(nsIPresContext * 0x03143bf0, nsIRenderingContext
& {...}, nsIFrame * 0x02cf633c, const nsRect & {...}, const nsRect & {...},
const nsStyleBackground & {...}, const nsStyleBorder & {...}, int 0, int 0) line
2172 + 80 bytes
nsBoxFrame::Paint(nsBoxFrame * const 0x02cf633c, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1302 + 37 bytes
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02cf633c, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02cf61a0, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02cf61a0, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02cacf58, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02cacf58, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02c9ee10, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02c9ee10, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02c9ed2c, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02c9ed2c, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02c9eb98, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02c9eb98, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02c80454, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsBoxFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext & {...},
const nsRect & {...}, nsIFrame * 0x02c80454, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1388
nsBoxFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
1524
nsBoxFrame::Paint(nsBoxFrame * const 0x02c801b8, nsIPresContext * 0x03143bf0,
nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 1342
nsRootBoxFrame::Paint(nsRootBoxFrame * const 0x02c801b8, nsIPresContext *
0x03143bf0, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 263
nsContainerFrame::PaintChild(nsIPresContext * 0x03143bf0, nsIRenderingContext &
{...}, const nsRect & {...}, nsIFrame * 0x02c801b8, nsFramePaintLayer
eFramePaintLayer_Underlay) line 229
nsContainerFrame::PaintChildren(nsIPresContext * 0x03143bf0, nsIRenderingContext
& {...}, const nsRect & {...}, nsFramePaintLayer eFramePaintLayer_Underlay) line
173
nsContainerFrame::Paint(nsContainerFrame * const 0x02c8017c, nsIPresContext *
0x03143bf0, nsIRenderingContext & {...}, const nsRect & {...}, nsFramePaintLayer
eFramePaintLayer_Underlay) line 155
PresShell::Paint(PresShell * const 0x03144c14, nsIView * 0x03143330,
nsIRenderingContext & {...}, const nsRect & {...}) line 5247 + 34 bytes
nsView::Paint(nsView * const 0x03143330, nsIRenderingContext & {...}, const
nsRect & {...}, unsigned int 128, int & 268601941) line 275
nsViewManager::RenderDisplayListElement(DisplayListElement2 * 0x042343d0,
nsIRenderingContext & {...}) line 1438
nsViewManager::RenderViews(nsIView * 0x03143330, nsIRenderingContext & {...},
const nsRect & {...}, int & 0) line 1363
nsViewManager::Refresh(nsIView * 0x03143330, nsIRenderingContext * 0x04233a50,
const nsRect * 0x0012f65c, unsigned int 1) line 900
nsViewManager::DispatchEvent(nsViewManager * const 0x031434d0, nsGUIEvent *
0x0012f79c, nsEventStatus * 0x0012f6a0) line 1957
HandleEvent(nsGUIEvent * 0x0012f79c) line 68
nsWindow::DispatchEvent(nsWindow * const 0x031431f4, nsGUIEvent * 0x0012f79c,
nsEventStatus & nsEventStatus_eIgnore) line 712 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f79c, nsEventStatus &
nsEventStatus_eIgnore) line 738
nsWindow::OnPaint() line 4003 + 28 bytes
nsWindow::ProcessMessage(unsigned int 15, unsigned int 0, long 0, long *
0x0012fbd0) line 2998 + 17 bytes
nsWindow::WindowProc(HWND__ * 0x00310370, unsigned int 15, unsigned int 0, long
0) line 979 + 27 bytes
USER32! 77e71ab7()
USER32! 77e71a77()
Reporter | ||
Comment 1•23 years ago
|
||
This is valeski's fault; only my windows build shows this, because I picked up
valeski's changes there.
This is a pretty bad performance regression.
Assignee: morse → valeski
OS: Windows NT → All
Hardware: PC → All
Comment 2•23 years ago
|
||
gone. this was part of my checkin to 84162. sorry :-/.
Reporter | ||
Comment 4•23 years ago
|
||
Reopening. I'm still seeing this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Trying to open cookperm.txt for every XUL image load → Trying to open cookperm.txt for every image load
Reporter | ||
Comment 5•23 years ago
|
||
-> morse. This is the image blocking code.
Assignee: valeski → morse
Status: REOPENED → NEW
Reporter | ||
Comment 6•23 years ago
|
||
For *every* image load, we are hitting code the cookie DLL for image blocking.
This involves reading in the cookperm.txt file (if it exists) every time!
This happens because the "imageblocker.enabled" pref is on by default:
http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/init/all.js#351
which blizzard changed as a purported fix for bug 35981.
Even when the pref is on, and the user has set their prefs to accept all images,
we hit cookperm.txt. Why don't we bail in the 'PERMISSION_Accept' case? This
logic lives in IMAGE_CheckForPermission().
nav triage team:
Hey-zoos, this sucks, but won't get to this soon. Marking this p2 and mozilla1.0
Priority: -- → P2
Target Milestone: --- → mozilla1.0
Reporter | ||
Comment 8•23 years ago
|
||
This is such an easy kill that is should be brought back to 0.9.2 I think. This
bug causes an extra stat and/or file read for every image that we load (cached or
not).
Attaching a patch which needs to be checked for sanity, but fixes the most common
case.
Reporter | ||
Comment 9•23 years ago
|
||
Reporter | ||
Comment 10•23 years ago
|
||
The patch adds a few lines which bail before the file read if your image loading
prefs are set to accept all images (PERMISSION_Accept).
Assignee | ||
Comment 11•23 years ago
|
||
The patch that Simon attached would be only a band-aid. The real fix is for the
permission module to keep track of the fact that it already tried to open
cookperm and not try again.
The logic in the permission module is to set up a datastructure the first time
that a permission is needed and to read the contents of cookperm into that
datastructure. On future requests for permission, a check is made for the
existence of the datastructure, and if it exists then the cookperm file is not
re-read.
Apparently if cookperm doesn't exist, the datastructure doesn't get set up and
so future requests for permissions will attempt to read cookperm again. The
correct fix is to set up the datastructure whether or not cookperm exists.
Reporter | ||
Comment 12•23 years ago
|
||
But why do we have to load the file at all if the user set prefs to accept all
images?
Assignee | ||
Comment 13•23 years ago
|
||
Probably because the permission module tests for the permission setting before
it tests the pref setting. The ordering of these tests should not be a big deal
except for the very first time when the cookperm file gets read in. And that
should happen only once, if the logic was working properly.
Assignee | ||
Comment 15•23 years ago
|
||
I have a patch that fixes this the correct way. sfraser, please review. cc'ing
alecf for super-review.
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla0.9.1
Assignee | ||
Comment 16•23 years ago
|
||
Comment 17•23 years ago
|
||
change that to NS_ERROR_OUT_OF_MEMORY and you've got yourself an sr=alecf
Reporter | ||
Comment 18•23 years ago
|
||
r=sfraser
Opportunities for later optimization:
Cache the "imageblocker.enabled" pref.
Do early return if the pref is set to 'PERMISSION_Accept'.
Cache the string bundle inside of CKutil_Localize().
Comment 19•23 years ago
|
||
Not holding 091 for this perf fix. -> 092
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Comment 20•23 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Assignee | ||
Comment 21•23 years ago
|
||
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•