Open Bug 1455024 Opened 6 years ago Updated 2 years ago

Simplify internal representation of nsID

Categories

(Core :: XPCOM, enhancement, P3)

enhancement

Tracking

()

Tracking Status
firefox61 --- affected

People

(Reporter: erahm, Unassigned)

References

Details

Currently `nsID` (aka nsIID/nsCID) is split into 4 member variables for it's internal representation [1]. AFAICT these separate portions are no longer used in a meaningful way. I'd like to propose we just switch to a 16-byte array instead. This would simplify various bits of comparison, serialization, conversion, and hashing logic.

[1] https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/xpcom/base/nsID.h#27-30
Depends on: 1444745
Two things to think about:

1) The structure needs to be 32-bit aligned so Equals() works correctly.
2) We serialize IIDs into various on-disk places, so whatever is done to nsID needs to be very careful that the IIDs read from disk continue to be the same.
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
(In reply to Nathan Froyd [:froydnj] from comment #1)
> Two things to think about:
> 
> 1) The structure needs to be 32-bit aligned so Equals() works correctly.

We could just make that a memcmp though...OTOH if we wanted to go crazy we could required 128-bit alignment and possibly get a nice speedup on platforms that support SSE2. We'd have to audit where nsID/nsIID/nsCID is actually used as a member variable to see how problematic that would be spacewise.

> 2) We serialize IIDs into various on-disk places, so whatever is done to
> nsID needs to be very careful that the IIDs read from disk continue to be
> the same.

AFAICT we serialize in big-endian [1], so if we store in-memory in big-endian we'd be okay (and it would make string parsing simpler too). Do you know of other places where we serialize manually?

[1] https://searchfox.org/mozilla-central/rev/14578d6f2d2ec5246572827061ecefa60a40855d/xpcom/io/nsBinaryStream.cpp#355-379
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.