Closed
Bug 78032
Opened 24 years ago
Closed 18 years ago
++iter optimizes better than iter++, fix callers (in tight loops)
Categories
(Core :: XPCOM, defect, P3)
Core
XPCOM
Tracking
()
RESOLVED
WORKSFORME
Future
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
Attachments
(2 files)
2.37 KB,
patch
|
Details | Diff | Splinter Review | |
9.70 KB,
patch
|
Details | Diff | Splinter Review |
As jag pointed out in bug 65431 and I tested, the statement |++iter| optimizes
better, at least on gcc 2.96-76 with -O2, than |iter++|. We should probably
convert tight loops that use string iterators (such as the ones in
nsScanner::ReadUntil) to use the prefix form, or we should figure out how to
make the compiler optimize the postfix as well as the prefix.
See attachment 32389 [details], "differences between prefix and postfix ++ with
gcc-2.96-76 -O2".
Comment 1•24 years ago
|
||
Of _course_ pre-increment is better! That's why I'm always pushing people to
develop the habit of preferring pre-increment over post-increment except when
demanded by idiom. This is a well known rule of C++. See "Effective C++" for
details. Post-increment typically requires a temporary (when implementing
standard behavior for user-defined types) which the C++ compiler can almost
never optimize away if you really are in a situation where post-increment is needed.
There are two reasons to prefer pre-increment. First: ``say what you mean''.
What you mean is ``increment this variable''. Not ``tell me the old value, then
increment this variable''. Second: the efficiency concern with respect to
user-defined types, particular complex iterators, e.g., for mozilla strings or
for containers from the standard C++ library. Consider other differences as
well, the result of post-increment must be a |const| rvalue, while the result of
pre-increment is a reference.
++ ++i // is legal for any built-int type, and should be for a user-defined
i++ ++ // turns out _not_ to be legal for built-in types
post-increment should be considered harmful except where demanded by idiom
Assignee | ||
Comment 2•24 years ago
|
||
Hmm. Maybe we should start writing in ++C...
Comment 3•24 years ago
|
||
So should this be resolved invalid? Too bad we can't resolve it Documentation...
Assignee | ||
Comment 4•24 years ago
|
||
No, we should fix callers. That was what this bug was about...
I was thinking we might even be able to come up with some debug hack that
asserted when iter++ was used and the value dropped (such as having the iter++
return a struct wrapping the new iterator (via private inheritance?) with an
operator to cast it to the iterator, and an assertion in the destructor of the
struct if the cast operator was never called).
Comment 5•24 years ago
|
||
You're right. I didn't re-read the first comment and I associated this bug with
just the "we should figure out how to make the compiler optimize the postfix as
well as the prefix" part.
Changing summary.
Summary: ++iter optimizes better than iter++ → ++iter optimizes better than iter++, fix callers (in tight loops)
Assignee | ||
Updated•24 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.2
Assignee | ||
Comment 6•24 years ago
|
||
Assignee | ||
Comment 7•24 years ago
|
||
Comment 8•24 years ago
|
||
r=peterv for the change to nsStyleLinkElement.cpp.
Comment 9•24 years ago
|
||
sr=scc for the stuff in the strings directory
Comment 10•24 years ago
|
||
r=harishd
Comment 11•24 years ago
|
||
r=dmose@netscape.com for the LDAP changes
Assignee | ||
Updated•24 years ago
|
OS: Linux → All
Hardware: PC → All
Comment 12•24 years ago
|
||
sr=hyatt
Comment 13•24 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
Assignee | ||
Comment 14•24 years ago
|
||
Patch checked in 2001-06-19 15:38 PDT.
Moving to future so I can go through this exercise again at some point in the
future.
Priority: P1 → P2
Target Milestone: mozilla0.9.2 → Future
Assignee | ||
Updated•23 years ago
|
Priority: P2 → P3
Comment 15•22 years ago
|
||
Comment on attachment 38762 [details] [diff] [review]
patch to break postfix ++ and -- so the compiler can find uses (not for checkin)
Even if this patch is for "debug use only" and not "trunk checkin",
In <nsScanner.cpp>, Shoudn't |mCountRemaining--;| be converted to
|--mCountRemaining;| ?
For obvious reason ;->
Assignee | ||
Comment 16•18 years ago
|
||
Should no longer be a problem with new stirng code.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
Updated•4 years ago
|
Component: String → XPCOM
You need to log in
before you can comment on or make changes to this bug.
Description
•