Closed Bug 783016 Opened 12 years ago Closed 12 years ago

Make space for Int32 string type

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: evilpie, Assigned: evilpie)

References

Details

Attachments

(1 file)

Attached patch v1 (deleted) — Splinter Review
So the basic change here is that replaced the encoding of Ropes with 0b000, so we can still test for non-linear/ropeness with two machine instructions. I also adjusted how we test for atoms, because I am going to need some easy way to create an Int32Atom. I also added one single bit for HasBase (dependent/undepended) strings, if we ever need more bits we could change that.
Attachment #652137 - Flags: review?(luke)
Comment on attachment 652137 [details] [diff] [review] v1 Review of attachment 652137 [details] [diff] [review]: ----------------------------------------------------------------- Nicely done! Good job shuffling the bits around :) So, IIUC, there will be two two varieties of Int32 strings: atomized and non-atomized. Is the idea that making a non-atomized string is much faster (no hash-lookup required) and, when we do atomize, we'd rather not convert to chars? Will there be two hash maps: chars -> non-int-atoms, int -> int-atoms? ::: js/src/vm/String.h @@ +178,5 @@ > * the predicate used to query whether a JSString instance is subtype > * (reflexively) of that type. > * > + * Rope 0000 0000 > + * Linear - xxxx To make the case analysis order-independent, could you change 'xxxx' to '!0000' @@ +179,5 @@ > * (reflexively) of that type. > * > + * Rope 0000 0000 > + * Linear - xxxx > + * HasBase - xxx1 Good idea including this in the table to make the assumption clear. Unfortunately, we now need to document "HasBase" ;-) How about after this table you just have a comment to the effect of "'HasBase' here refers to the two string types that have a 'base' field: JSDependentString and JSUndependedString. A JSUndependedString is a JSDependentString which has been 'fixed' (by ensureFixed) to be null-terminated. In such cases, the string must keep marking its base since there may be any number of *other* JSDependentStrings transitively depending on it.". @@ +352,5 @@ > } > > JS_ALWAYS_INLINE > bool isAtom() const { > + return !!(d.lengthAndFlags & ATOM_BIT); I'd be happy to leave out the !!. (I think we've turned off the annoying msvc warning.) @@ +365,5 @@ > /* Only called by the GC for dependent or undepended strings. */ > > inline bool hasBase() const { > + JS_STATIC_ASSERT((DEPENDENT_FLAGS | JS_BIT(1)) == UNDEPENDED_FLAGS); > + return !!(d.lengthAndFlags & HAS_BASE_BIT); ditto
Attachment #652137 - Flags: review?(luke) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: