Closed Bug 1381084 Opened 7 years ago Closed 7 years ago

imgRequest::BoostPriority happens way too late (image network load prioritization doesn't work at all)

Categories

(Core :: Graphics: ImageLib, enhancement, P1)

enhancement

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mayhemer, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: gfx-noted, feature)

According logs this happens way after we get OnStart/OnData from the child channel. Hence, it renders work in bug 1312771 and related to void.
There may not be much we can do about this; there isn't really anything earlier to tell when an image is visible.
Whiteboard: gfx-noted, feature
(In reply to Timothy Nikkel (:tnikkel) from comment #1) > There may not be much we can do about this; there isn't really anything > earlier to tell when an image is visible. Timothy, do you have any idea how we could figure out any kind of priority before we AsyncOpen the loading channel? At least we should know if the tag has size (width= at least) soon enough. It's a bit hard for me to believe we at least don't have the information an image tag appears in the view port when we scroll. Changing a priority of an already scheduled network request would work in that case. What does the patch in bug 1312771 actually do?
Flags: needinfo?(tnikkel)
Summary: imgRequest::BoostPriority happens way too late → imgRequest::BoostPriority happens way too late (image network load prioritization doesn't work at all)
(In reply to Honza Bambas (:mayhemer) from comment #2) > Timothy, do you have any idea how we could figure out any kind of priority > before we AsyncOpen the loading channel? At least we should know if the tag > has size (width= at least) soon enough. If the dom node has the width/height attributes we could get those. But there is also the case that the images size is specified via CSS, and CSS styles can be resolved before frames are constructed but they usually aren't. If we haven't created the frames for the document and reflowed them we have no idea if the image is visible or not. There isn't much we can do. > It's a bit hard for me to believe we at least don't have the information an > image tag appears in the view port when we scroll. Changing a priority of > an already scheduled network request would work in that case. What does the > patch in bug 1312771 actually do? If the page is visible on screen then we would have the information necessary, but there's a good chance the network request is already finished.
Flags: needinfo?(tnikkel)
@mayhemer, is your test case trying to load image via <img> that written in the HTML? HTML5 parser will preload the image URL soon after create the corresponding DOM element, which I believe will be way earlier than nsImageFrame initialization. https://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/parser/html/nsHtml5TreeBuilderCppSupplement.h#136 https://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/parser/html/nsHtml5TreeOpExecutor.cpp#978 If this is the case we want to optimize as well, https://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/image/imgLoader.cpp#2265 might be the place to adjust the priority. |request| is the corresponding imgRequest and |aContext| is the DOM element which we can check if size is available.
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #4) > @mayhemer, is your test case trying to load image via <img> that written in > the HTML? > HTML5 parser will preload the image URL soon after create the corresponding > DOM element, which I believe will be way earlier than nsImageFrame > initialization. > https://searchfox.org/mozilla-central/rev/ > a83a4b68974aecaaacdf25145420e0fe97b7aa22/parser/html/ > nsHtml5TreeBuilderCppSupplement.h#136 > https://searchfox.org/mozilla-central/rev/ > a83a4b68974aecaaacdf25145420e0fe97b7aa22/parser/html/nsHtml5TreeOpExecutor. > cpp#978 yes, you are right! > > If this is the case we want to optimize as well, > https://searchfox.org/mozilla-central/rev/ > a83a4b68974aecaaacdf25145420e0fe97b7aa22/image/imgLoader.cpp#2265 might be > the place to adjust the priority. > |request| is the corresponding imgRequest and |aContext| is the DOM element > which we can check if size is available. we could at least implement this. are you willing to write a patch? I would prefer opening a new bug just for that.
Flags: needinfo?(schien)
(In reply to Timothy Nikkel (:tnikkel) from comment #3) > (In reply to Honza Bambas (:mayhemer) from comment #2) > > Timothy, do you have any idea how we could figure out any kind of priority > > before we AsyncOpen the loading channel? At least we should know if the tag > > has size (width= at least) soon enough. > > If the dom node has the width/height attributes we could get those. But > there is also the case that the images size is specified via CSS, Yep, I'm aware, but I believe giving a priority to tags w/o w/h attributes may give us a small win. > and CSS > styles can be resolved before frames are constructed but they usually aren't. > > If we haven't created the frames for the document and reflowed them we have > no idea if the image is visible or not. There isn't much we can do. Understand. > > > It's a bit hard for me to believe we at least don't have the information an > > image tag appears in the view port when we scroll. Changing a priority of > > an already scheduled network request would work in that case. What does the > > patch in bug 1312771 actually do? > > If the page is visible on screen then we would have the information > necessary, but there's a good chance the network request is already finished. Sounds like we need some study being made here, I'd be interested in how often we find out an image has already been loaded from the net but is not on the screen. I'll file a new bug for it.
(In reply to Honza Bambas (:mayhemer) from comment #5) > (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment > #4) > > @mayhemer, is your test case trying to load image via <img> that written in > > the HTML? > > HTML5 parser will preload the image URL soon after create the corresponding > > DOM element, which I believe will be way earlier than nsImageFrame > > initialization. > > https://searchfox.org/mozilla-central/rev/ > > a83a4b68974aecaaacdf25145420e0fe97b7aa22/parser/html/ > > nsHtml5TreeBuilderCppSupplement.h#136 > > https://searchfox.org/mozilla-central/rev/ > > a83a4b68974aecaaacdf25145420e0fe97b7aa22/parser/html/nsHtml5TreeOpExecutor. > > cpp#978 > > yes, you are right! > > > > > If this is the case we want to optimize as well, > > https://searchfox.org/mozilla-central/rev/ > > a83a4b68974aecaaacdf25145420e0fe97b7aa22/image/imgLoader.cpp#2265 might be > > the place to adjust the priority. > > |request| is the corresponding imgRequest and |aContext| is the DOM element > > which we can check if size is available. > > we could at least implement this. are you willing to write a patch? I > would prefer opening a new bug just for that. Bug 1382567 is filed and the WIP patch is attached.
Flags: needinfo?(schien)
I'm afraid we can't do much more than bug 1382567.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.