Closed
Bug 168613
Opened 22 years ago
Closed 13 years ago
Turn PtrBits into a class
Categories
(Core :: XPCOM, defect, P4)
Tracking
()
RESOLVED
INCOMPLETE
mozilla1.3beta
People
(Reporter: john, Unassigned)
Details
(Whiteboard: [FIX])
Attachments
(1 file, 4 obsolete files)
25.78 KB,
patch
|
Details | Diff | Splinter Review |
The error-prone PtrBits abstraction, which reuses the empty bottom 2 bits of a
pointer to an object (or a malloced pointer), is spreading through Mozilla like
wildfire. This is good because it can save a significant amount of space
through treating the PtrBits like a union with the lower bits telling us what
the higher bits in the pointer mean.
Since we are using it a lot, we should formalize it. This will also protect us
against architectures in the future where the bottom 2 bits are *not* always
zero--in such an architecture we can change this class to include a boolean and
all will be well. We can make a fairly simple abstraction which captures the
heart of this by storing a union with 2 or 4 void*'s and Int32s.
Formalizing will also encourage use--I know I have sometimes decided to go the
easy route of creating a PRInt32 flags or added a second pointer when I could
have done a union and saved space. With such a class it would be easy enough to
find an existing pointer, change it to PtrBits, and reuse unused bits.
I will upload a file shortly with some classes to tinker with and think about.
Reporter | ||
Comment 1•22 years ago
|
||
This has not been tested or compiled, but shows the shape of things to come.
What do y'all think?
Reporter | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [FIX]
Reporter | ||
Comment 2•22 years ago
|
||
That one had SetPtr() wrong. But the idea is the same, and the interface is
the main thing right now :)
Attachment #99183 -
Attachment is obsolete: true
Reporter | ||
Comment 3•22 years ago
|
||
scc, I'm curious where you see this going too. xpcom/base?
Reporter | ||
Comment 4•22 years ago
|
||
CC'ing others with a potential interest in the PtrBits abstraction.
Reporter | ||
Comment 5•22 years ago
|
||
OK, that one was sorta bogus. I'm going to start small with a PtrFlags class
with an interface like this:
class PtrFlags {
public:
void* get();
operator=(void* aFlags);
PRBool GetFlag1();
PRBool GetFlag2();
void SetFlag1(PRBool aFlag);
void SetFlag2(PRBool aFlag);
};
And then graduate (hopefully) to:
template<T>
class PtrFlags {
public:
T* get();
T* operator->();
PtrFlags& operator=(T* aFlags);
PRBool GetFlag1();
PRBool GetFlag2();
void SetFlag1(PRBool aFlag);
void SetFlag2(PRBool aFlag);
void Assign(T* aPtr, PRBool aFlag1, PRBool aFlag2);
};
Union classes can use this as a jumping point.
Thoughts?
Comment 6•22 years ago
|
||
The big drawback this has is that Flag1 and Flag2 are rather generic, making
code less readable when used.
Reporter | ||
Comment 7•22 years ago
|
||
Yes, if a class wants to implement a helper function to get flag1 and flag2 with
a better name it can and probably should; I didn't want to go and make a
macro-based class when the user of the class can just as easily do it
themselves. What this buys a user is freedom from brittle & and | statements
and potentially allows them to work on other architectures if an architecture
without the word alignment "feature" comes along.
Comment 8•22 years ago
|
||
How about making a macro which makes the flag bits more readable:
NS_DECL_PTRBITS(mMyFlagPtrBits, Foo, Bar);
which would expand to something like
class mMyFlagPtrBitsClass : public nsPtrBits {
PRBool getFoo() { return GetFlag1(); }
PRBool getBar() { return GetFlag2(); }
(with setFoo/etc)
};
mMyFlagPtrBitsClass mMyFlagPtrBits;
Though I'd like to suggest that instead of PRBool, you use PRPackedBool, because
if you're already space consious, you might be assigning this into a
PRPackedBool in another class, and you could avoid the inflation and deflation..
Reporter | ||
Comment 9•22 years ago
|
||
This is by no means the final version, but this one has everything one would
want in such an interface. Alec, the macro idea sounds good but I'd like to
explore how it's going to be used--nsCheapSets.h, for example, needs to do more
than just get/set, it is dynamically figuring out what to put in there.
TODO:
- templatize!
- investigate macros
- make it possible to also store / retrieve PRInt32 (a common use of PtrBits is
to switch between storing a pointer and storing an int)
Attachment #99184 -
Attachment is obsolete: true
Reporter | ||
Comment 10•22 years ago
|
||
This is a templatized version that does, for example, nsPtrBits<nsIForm>. It
also supports Get/SetInt() and Get/SetInt2(), which allow you to store unsigned
integers in the PtrBits as well (a thing we do in a couple of places at least).
Finally, it has regression tests.
I'll start converting some places over that use PtrBits tomorrow.
Attachment #99475 -
Attachment is obsolete: true
Reporter | ||
Comment 11•22 years ago
|
||
This makes a little templatized nsPtrBits and uses them in nsGenericElement.*.
scc, could you look it over? I have some testing to do on my end--specifically
compile on Mac and Linux--but I sure could use input on templates in general.
Attachment #99490 -
Attachment is obsolete: true
Reporter | ||
Updated•22 years ago
|
Priority: -- → P4
Target Milestone: --- → mozilla1.3beta
Updated•19 years ago
|
Assignee: john → nobody
Status: ASSIGNED → NEW
QA Contact: scc → xpcom
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•