Bug 1017079 - Fix obsolete comment in AtomizeAndCopyChars. r=bhackett.
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 05 Jun 2014 13:23:40 -0400
changeset 207527 c48f50085fecd32758628f3898ff3c02f8674209
parent 207526 ad465c247417bfb14250ad59f3a10e7616a190d7
child 207528 3825abd9a302a6270bd41f83bf666313af5cd758
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1017079
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1017079 - Fix obsolete comment in AtomizeAndCopyChars. r=bhackett.
js/src/jsatom.cpp
--- a/js/src/jsatom.cpp
+++ b/js/src/jsatom.cpp
@@ -360,23 +360,16 @@ AtomizeAndCopyChars(ExclusiveContext *cx
          return s;
 
     AtomHasher::Lookup lookup(tbchars, length);
 
     AtomSet::Ptr pp = cx->permanentAtoms().readonlyThreadsafeLookup(lookup);
     if (pp)
         return pp->asPtr();
 
-    /*
-     * If a GC occurs at js_NewStringCopy then |p| will still have the correct
-     * hash, allowing us to avoid rehashing it. Even though the hash is
-     * unchanged, we need to re-lookup the table position because a last-ditch
-     * GC will potentially free some table entries.
-     */
-
     AutoLockForExclusiveAccess lock(cx);
 
     AtomSet& atoms = cx->atoms();
     AtomSet::AddPtr p = atoms.lookupForAdd(lookup);
     if (p) {
         JSAtom *atom = p->asPtr();
         p->setTagged(bool(ib));
         return atom;
@@ -387,17 +380,20 @@ AtomizeAndCopyChars(ExclusiveContext *cx
     JSFlatString *flat = js_NewStringCopyN<NoGC>(cx, tbchars, length);
     if (!flat) {
         js_ReportOutOfMemory(cx);
         return nullptr;
     }
 
     JSAtom *atom = flat->morphAtomizedStringIntoAtom();
 
-    if (!atoms.relookupOrAdd(p, lookup, AtomStateEntry(atom, bool(ib)))) {
+    // We have held the lock since looking up p, and the operations we've done
+    // since then can't GC; therefore the atoms table has not been modified and
+    // p is still valid.
+    if (!atoms.add(p, AtomStateEntry(atom, bool(ib)))) {
         js_ReportOutOfMemory(cx); /* SystemAllocPolicy does not report OOM. */
         return nullptr;
     }
 
     return atom;
 }
 
 JSAtom *