277483 - Kill nsIAtom?

Status: NEW

(Core :: XPCOM, defect, P3)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Something I've pondered for a that we talked about in #developers today was the
possibility of getting rid of nsIAtom. Once darin has made it possible to use
the refcounted buffer in strings (bug 235499) there will be very little
difference between these buffers and atoms. We could have a hashtable with
stringbuffers keyed on their value just like with atoms.

The advantage of this is that it would be very cheap and easy to convert an atom
into a string. It would also make it cheaper to create a new atom since no
string-data would need to be allocated or copied (actually, this could be done
even if we keep nsIAtom). Hmm.. there are problaby other advantages too, but
it's getting too late here. Bsmedberg seemed to have some stuff in mind?

There are a few ticky things with this. We've used some tricks on nsIAtoms that
would be hard to do on stringbuffers. In particular permanent atoms that aren't
refcounted (to avoid thread-safe refcounts), and statical atoms (which uses
static stringdata).

This might or might not be a good idea at all, please raise oppinions.

Comment 1

[Darin Fisher]

permanent atoms can be dealt with easily.  remember, a nsString has a mFlags
member which is a bit-field specifying the storage type among other things.  so,
it is possible to distinguish ref-counted atoms and permanent atoms.  in fact,
permanent atoms are a lot like dependent strings, except that copying from one
string to another is much more efficient.

the downside to using nsString for atoms is that you lose the ability to cheaply
compare two atoms.  comparing nsString's means calling Equals.  even if we
optimize Equals to compare the address of mData first, we still have the
function call overhead.  this is the biggest drawback to this proposal.

that said, i think there may be something here.  perhaps we can at least make
"assignment" between nsIAtom and nsString more efficient.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

My idea was that we would have something like

class nsAtomizedString : public nsString
{
  PRBool FastCompare(const nsAtomizedString& aOther)
  {
    return mData == aOther.mData;
  }
}

And then we'd pass around these rather then nsIAtoms

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #0)
> The advantage of this is that it would be very cheap and easy to convert
> an atom into a string.

There are many other ways to do that, such as keeping atoms but having them keep
strings inside of them.  (I had most of a patch for this written a few years
ago; it's probably in a bug somewhere, but it's the old string API, so probably
not much use.)

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> the downside to using nsString for atoms is that you lose the ability to cheaply
> compare two atoms.

That would be a disaster, must be avoided.

Comment 5

[Benjamin Smedberg]

Hrm, this idea is kinda spread out and unspeced, but I think it is absolutely
essential to get rid of the nsIAtom interface. It's blatantly against the COM
rules, it can't be remoted (or if it is it loses its special non-COM identity
properties).

We should have an atom table similar to the way .NET does interned strings. Any
const ACString can be "interned" to return an atom that can then be
pointer-compared and switch()ed, instead of string-compared. Then, any atom can
be passed around as a regular const ACString.

Furthermore, I think we should do the same thing for AString. We would keep
another atom table for wide strings. You obviously cannot compare a narrow atom
and a wide atom, as they are different types. But it seems from discussions with
some DOM folk that some DOM operations would be simplified if we had unicode atoms.

Comment 6

[Darin Fisher]

bear in mind that we cannot "pointer compare" nsAString instances directly so
long as we still have to support pre- mozilla 1.7 nsAString.  we'd still have to
check for the canonical vtable, etc. as is done with all nsAString methods.

Comment 7

[Alec Flett]

I like comment 2's idea, or even something simpler making an atom structure that
does not derive from anything. atoms are fine concepts but
1) there's no real need for it to be an interface, aside from scriptability, so
ditching nsIAtom in favor of some concrete "nsAtom" seems ok. (and to counter
the scriptability issue - if you're relying on pointer comparisons for speed in
scripted code, you're probably doing the wrong thing anyway - we could/should
just convert consumers to be using raw strings)

2) I don't see any great advantage to making it derive from nsAString - its not
a string, its an atom. An atom should mostly be compared to compared to atoms
Making some syntactical sugar which makes it easy to compare it to strings would
just make people abuse the comparison. It should be explicit. I would favor
atom.EqualsString(const nsAString&) over atom.Equals(const nsAString&) for example.

Comment 8

[Doug Turner (:dougt)]

mass reassigning to nobody.