271386 - nsHttpResponseHead: Some functions can be made private...

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P5)

Description

[Alfred Kayser]

Functions like ParseDateHeader GetDateValue, etc, are only used by
nsHttpResponseHead.cpp itself, so these can be made private, which
allows the compiler to further optimize these.

http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/http/src/nsHttpResponseHead.h#125
125     // these return failure if the header does not exist.
126     nsresult ParseDateHeader(nsHttpAtom header, PRUint32 *result);
127     nsresult GetAgeValue(PRUint32 *result);
128     nsresult GetMaxAgeValue(PRUint32 *result);
129     nsresult GetDateValue(PRUint32 *result)         { return
ParseDateHeader(nsHttp::Date, result); }
130     nsresult GetExpiresValue(PRUint32 *result);
131     nsresult GetLastModifiedValue(PRUint32 *result) { return
ParseDateHeader(nsHttp::Last_Modified, result); }

Comment 1

[Darin Fisher]

How does marking them private improve compiler optimization?  Marking them
NS_HIDDEN would certainly help.

Comment 2

[Alfred Kayser]

At least they will be hidden from misuse by others, 
and let's use NS_HIDDEN if that will help

Comment 3

[Alfred Kayser]

Note that GetDateValue and GetLastModifiedValues are both 'one liners' inline defined in the class definition, so if they are private the compiler can then really inline them if the compiler wants to do so.

Also GetAgeValue and GetMaxAgeValue are relatively short functions, and only used in one location, so if they are private the compiler can inline them in the code if it helps optimization. 

So, by just defining these as 'private' will help the compiler to do its optimization job.

Comment 4

[Steven Laabs [:Riposte]]

I'd like to take this one if possible.

Comment 5

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 7

[Somdatta]

Hello,
I am Somdatta and I am an outreachy applicant.I wanted to know whether this bug has been resolved or not.If not,I would like to work on it.
Thanks

Comment 8

[Jens Stutte [:jstutte]]

While some of the functions changed slightly, the general intent seems still valuable here, IMHO. Dragana, wdyt?

Comment 9

[Dragana Damjanovic [:dragana]]

Please take it and ping me if you need help.

Comment 10

[Somdatta]

Okay.I am working on it then.
One question,who should I submit a patch for review to?

Comment 11

[Jens Stutte [:jstutte]]

Please follow this guide on how to submit a patch with moz-phab. As reviewer you can put r?#necko-reviewers into the commit message. Thanks!

Comment 12

[Somdatta]

okay

Comment 13

[Somdatta]

Attachment: Bug 271386- Make some functions private in nsHttpResponseHead r?#necko-reviewers

Make the following functions private because they are only used in nsHttpResponseHead.cpp for compiler optimization-
GetDateValue,GetAgeValue,GetMaxAgeValue,GetExpiresValue.
GetLastModifiedValue cannot be made private since it is being used in CachePushChecker.cpp and ParseDateHeader is already private.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a747b8d10c6b
Make some functions private in nsHttpResponseHead r=necko-reviewers,kershaw

Comment 15

[Somdatta]

Just wanted to confirm..the submitted patch has been landed right?

Comment 16

[Jens Stutte [:jstutte]]

(In reply to Somdatta from comment #15)

Just wanted to confirm..the submitted patch has been landed right?

It is not yet in central. Now automated tests and other behind the scenes verification is running. You will see a second message here once it is there.

Comment 17

[Somdatta]

okay

Comment 18

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a747b8d10c6b