Is't wrong to store a GC thing on a map JSObject *

I want to have a map that carries rootedObject is that achievable?

typedef std::map <const std::string, JS::RootedObject> JSObjectsMap;
JSObjectsMap rootedObjectsMap;

```
bool MozillaJSEngine::addToRootedObjectsMap(const std::string& key, JS::Han=
dleObject value) {
    bool ok =3D true;
    JS::RootedObject obj(cx, value.get());
    if (!(rootedObjectsMap.insert(std::pair<std::string, JS::RootedObject>(=
key, obj)).second)) {
        printf("error adding object to rootedObjectsMap\n");
        ok =3D false;
    }

    return ok;
}
```

an Error appear on this line
 if (!(rootedObjectsMap.insert(std::pair<std::string, JS::RootedObject>(key=
, obj)).second))

IDE Message
```
 error: no matching function for call to =E2=80=98std::pair<std::__cxx11::b=
asic_string<char>, JS::Rooted<JSObject*> >::pair(const string&, JS::RootedO=
bject&)=E2=80=99

 note:   types =E2=80=98std::tuple<_Args1 ...>=E2=80=99 and =E2=80=98const =
string {aka const std::__cxx11::basic_string<char>}=E2=80=99 have incompati=
ble cv-qualifiers

 note:   types =E2=80=98std::pair<_T1, _T2>=E2=80=99 and =E2=80=98const str=
ing {aka const std::__cxx11::basic_string<char>}=E2=80=99 have incompatible=
 cv-qualifiers

 note:   =E2=80=98const string {aka const std::__cxx11::basic_string<char>}=
=E2=80=99 is not derived from =E2=80=98const std::pair<_T1, _T2>=E2=80=99

 note:   candidate expects 0 arguments, 2 provided

 note:   =E2=80=98JS::Rooted<JSObject*>=E2=80=99 is not derived from =E2=80=
=98std::tuple<_Args1 ...>```
0
msami
4/11/2019 1:13:02 PM
mozilla.dev.tech.js-engine 2038 articles. 0 followers. Post Follow

5 Replies
75 Views

Similar Articles

[PageSpeed] 4

On Thursday, April 11, 2019 at 3:13:04 PM UTC+2, msami wrote:
> I want to have a map that carries rootedObject is that achievable?
>=20
> typedef std::map <const std::string, JS::RootedObject> JSObjectsMap;
> JSObjectsMap rootedObjectsMap;
>=20
> ```
> bool MozillaJSEngine::addToRootedObjectsMap(const std::string& key, JS::H=
andleObject value) {
>     bool ok =3D true;
>     JS::RootedObject obj(cx, value.get());
>     if (!(rootedObjectsMap.insert(std::pair<std::string, JS::RootedObject=
>(key, obj)).second)) {
>         printf("error adding object to rootedObjectsMap\n");
>         ok =3D false;
>     }
>=20
>     return ok;
> }
> ```
>=20
> an Error appear on this line
>  if (!(rootedObjectsMap.insert(std::pair<std::string, JS::RootedObject>(k=
ey, obj)).second))
>=20
> IDE Message
> ```
>  error: no matching function for call to =E2=80=98std::pair<std::__cxx11:=
:basic_string<char>, JS::Rooted<JSObject*> >::pair(const string&, JS::Roote=
dObject&)=E2=80=99
>=20
>  note:   types =E2=80=98std::tuple<_Args1 ...>=E2=80=99 and =E2=80=98cons=
t string {aka const std::__cxx11::basic_string<char>}=E2=80=99 have incompa=
tible cv-qualifiers
>=20
>  note:   types =E2=80=98std::pair<_T1, _T2>=E2=80=99 and =E2=80=98const s=
tring {aka const std::__cxx11::basic_string<char>}=E2=80=99 have incompatib=
le cv-qualifiers
>=20
>  note:   =E2=80=98const string {aka const std::__cxx11::basic_string<char=
>}=E2=80=99 is not derived from =E2=80=98const std::pair<_T1, _T2>=E2=80=99
>=20
>  note:   candidate expects 0 arguments, 2 provided
>=20
>  note:   =E2=80=98JS::Rooted<JSObject*>=E2=80=99 is not derived from =E2=
=80=98std::tuple<_Args1 ...>```

so if I want to store JSObject in map and then set it on RootedObject will =
that cause problems on my application
0
msami
4/11/2019 1:22:28 PM
On 4/11/19 9:13 AM, msami wrote:
> I want to have a map that carries rootedObject is that achievable?

RootedObject is only allowed to be used on the stack.  So you can't 
store it in a map, no.

Depending on the rooting behavior you want, you could try to use a 
PersistentRooted for the value, or use a Heap<JSObject*> and trace your map.

In the former case, of course, you will need to deal with the fact that 
a PersistentRooted needs a JSContext or JSRuntime to be initialized.

-Boris
0
Boris
4/11/2019 4:09:11 PM
On 4/11/19 6:13 AM, msami wrote:
> I want to have a map that carries rootedObject is that achievable?

No, you don't want that. You really don't.

Rooted needs to be LIFO ordered, and unless you're really really careful 
with the order of hashtable operations, including internal resizes and 
things, that's never going to be LIFO. (And if it were, you wouldn't 
need the map.)

> typedef std::map <const std::string, JS::RootedObject> JSObjectsMap;
> JSObjectsMap rootedObjectsMap;

You can use std::map<const std::string, JS::PersistentRootedObject> 
instead. Though note that you'll need to construct these with a cx so 
they can register themselves in the root lists:

     using JSObjectsMap = std::map<const std::string, 
JS::PersistentRootedObject>;
     static JSObjectsMap rootedObjectsMap;
     ...

rooted.insert(JSObjectsMap::value_type(std::string("foo"), 
JS::PersistentRootedObject(cx, obj)));
       fprintf(stderr, "foo -> %p\n", (void*) rooted[std::string("foo")]);

You will also need to do rootedObjectsMap.clear() before shutting down. 
It is invalid to have any live objects at shutdown, and anything in a 
PersistentRooted is forever alive until you manually delete the 
PersistentRooted.


0
Steve
4/11/2019 5:45:06 PM
On 4/11/19 9:09 AM, Boris Zbarsky wrote:
> On 4/11/19 9:13 AM, msami wrote:
>> I want to have a map that carries rootedObject is that achievable?
>
> RootedObject is only allowed to be used on the stack.  So you can't 
> store it in a map, no.
>
> Depending on the rooting behavior you want, you could try to use a 
> PersistentRooted for the value, or use a Heap<JSObject*> and trace 
> your map.
>
> In the former case, of course, you will need to deal with the fact 
> that a PersistentRooted needs a JSContext or JSRuntime to be initialized.

Before I realized that PersistentRooted was fine for this, I went 
through these other options. Posting here for posterity. I should 
probably add them to the embedding examples.

----

There are a couple of options.

One would be to specialize templates appropriately to understand how to 
trace and sweep std::map. Then you could use 
PersistentRooted<std::map<std::string, JS::Heap<JSObject*>>>:

     namespace JS {
     template <typename NonGCKeyT, typename GCValueT>
     struct GCPolicy<std::map<NonGCKeyT, GCValueT>> {
       using Map = std::map<NonGCKeyT, GCValueT>;
       static void trace(JSTracer* trc, Map* map, const char* name) {
         for (auto& ent : *map) {
           GCPolicy<GCValueT>::trace(trc, &ent.second, "map value");
         }
       }
       static void sweep(Map* map) {
         auto iter = map->begin();
         while (iter != map->end()) {
           if (GCPolicy<GCValueT>::needsSweep(&iter->second))
             iter = map->erase(iter);
           else
             ++iter;
         }
       }
       static bool needsSweep(Map* map) {
         return !map->empty();
       }
       static bool isValid(Map* map) { return true; }
     };
     } /* namespace JS */

     using ObjMap = std::map<std::string, JS::Heap<JSObject*>>;
     static JS::PersistentRooted<ObjMap> rootedObjectsMap;

Note that this only suffices for maps with non-GC types as keys. If you 
were mapping from a JSObject*, for example, then our moving GC could 
invalidate the pointer and corrupt your std::map (and you can't just 
update the pointer there, because it'll hash differently, and anyway STL 
makes the key const so you don't do that.) The Heap<T> is on the key for 
the post-write barrier: if you put a nursery pointer into this map, we 
need to remember where it is so we can update it when the object moves. 
And if you're using incremental GC, it'll also give a pre-write barrier 
to prevent any pointers from hiding in the basement while the marking 
storm passes by overhead.

The sweep() implementation up there is not used in your case, since the 
only instance is rooted and thus everything will always be alive. But it 
seemed like a footgun to leave it out, and having it will allow 
JS::Heap<std::map<...>> (still for non-GC keys only, though.)

PersistentRooted is a tricky beast. You'll need to initialize it in some 
startup code, after you've constructed the runtime and everything, and 
you need to ensure that you reset() it before shutting down or we'll 
assert in a debug build (you'd be keeping objects live at shutdown, 
leaking random stuff and possibly preventing finalizers from running.)

During initialziation:

     rootedObjectsMap.init(cx);

and before everything shuts down:

     rootedObjectsMap.reset();

Using it would just be what you're used to:

   rooted.get().insert(ObjMap::value_type(std::string("foo"), obj));
   fprintf(stderr, "foo -> %p\n", (void*) rooted.get()[std::string("foo")]);

Another option would be to use our GC-aware map API, GCHashMap. But 
again, you'd have to do template stuff because we don't natively know 
how to hash std::string, nor do we know that it's not a GC pointer type 
so it doesn't need to be traced or die during GC:

     namespace mozilla {
     template <>
     struct DefaultHasher<std::string> {
       using Key = std::string;
       using Lookup = Key;

       static HashNumber hash(const Lookup& aLookup) {
         return HashString(aLookup.c_str());
       }

       static bool match(const Key& aKey, const Lookup& aLookup) {
         return aKey == aLookup;
       }
     };
     } /* namespace mozilla */

     namespace JS {
     template <>
     struct GCPolicy<std::string> : public IgnoreGCPolicy<std::string> {};
     } /* namespace JS */

     using ObjMap = JS::GCHashMap<std::string, JS::Heap<JSObject*>, 
js::DefaultHasher<std::string>, js::SystemAllocPolicy>;

The API is very different from std::map, though:

   rooted.put(std::string("foo"), obj);
   fprintf(stderr, "foo -> %p\n", (void*) 
rooted.lookup(std::string("foo"))->value());

0
Steve
4/11/2019 5:50:51 PM
I also offer up the examples here which try to explain this stuff. https://=
github.com/spidermonkey-embedders/spidermonkey-embedding-examples/blob/esr6=
0/examples/tracing.cpp

--Ted

On Thursday, April 11, 2019 at 1:51:05 PM UTC-4, Steve Fink wrote:
> On 4/11/19 9:09 AM, Boris Zbarsky wrote:
> > On 4/11/19 9:13 AM, msami wrote:
> >> I want to have a map that carries rootedObject is that achievable?
> >
> > RootedObject is only allowed to be used on the stack.=C2=A0 So you can'=
t=20
> > store it in a map, no.
> >
> > Depending on the rooting behavior you want, you could try to use a=20
> > PersistentRooted for the value, or use a Heap<JSObject*> and trace=20
> > your map.
> >
> > In the former case, of course, you will need to deal with the fact=20
> > that a PersistentRooted needs a JSContext or JSRuntime to be initialize=
d.
>=20
> Before I realized that PersistentRooted was fine for this, I went=20
> through these other options. Posting here for posterity. I should=20
> probably add them to the embedding examples.
>=20
> ----
>=20
> There are a couple of options.
>=20
> One would be to specialize templates appropriately to understand how to=
=20
> trace and sweep std::map. Then you could use=20
> PersistentRooted<std::map<std::string, JS::Heap<JSObject*>>>:
>=20
>  =C2=A0=C2=A0=C2=A0 namespace JS {
>  =C2=A0=C2=A0=C2=A0 template <typename NonGCKeyT, typename GCValueT>
>  =C2=A0=C2=A0=C2=A0 struct GCPolicy<std::map<NonGCKeyT, GCValueT>> {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 using Map =3D std::map<NonGCKeyT, GCValue=
T>;
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 static void trace(JSTracer* trc, Map* map=
, const char* name) {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 for (auto& ent : *map) {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 GCPolicy<GCValueT=
>::trace(trc, &ent.second, "map value");
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 static void sweep(Map* map) {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 auto iter =3D map->begin();
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 while (iter !=3D map->end()) =
{
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (GCPolicy<GCVa=
lueT>::needsSweep(&iter->second))
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iter =
=3D map->erase(iter);
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ++ite=
r;
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 static bool needsSweep(Map* map) {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return !map->empty();
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 static bool isValid(Map* map) { return tr=
ue; }
>  =C2=A0=C2=A0=C2=A0 };
>  =C2=A0=C2=A0=C2=A0 } /* namespace JS */
>=20
>  =C2=A0=C2=A0=C2=A0 using ObjMap =3D std::map<std::string, JS::Heap<JSObj=
ect*>>;
>  =C2=A0=C2=A0=C2=A0 static JS::PersistentRooted<ObjMap> rootedObjectsMap;
>=20
> Note that this only suffices for maps with non-GC types as keys. If you=
=20
> were mapping from a JSObject*, for example, then our moving GC could=20
> invalidate the pointer and corrupt your std::map (and you can't just=20
> update the pointer there, because it'll hash differently, and anyway STL=
=20
> makes the key const so you don't do that.) The Heap<T> is on the key for=
=20
> the post-write barrier: if you put a nursery pointer into this map, we=20
> need to remember where it is so we can update it when the object moves.=
=20
> And if you're using incremental GC, it'll also give a pre-write barrier=
=20
> to prevent any pointers from hiding in the basement while the marking=20
> storm passes by overhead.
>=20
> The sweep() implementation up there is not used in your case, since the=
=20
> only instance is rooted and thus everything will always be alive. But it=
=20
> seemed like a footgun to leave it out, and having it will allow=20
> JS::Heap<std::map<...>> (still for non-GC keys only, though.)
>=20
> PersistentRooted is a tricky beast. You'll need to initialize it in some=
=20
> startup code, after you've constructed the runtime and everything, and=20
> you need to ensure that you reset() it before shutting down or we'll=20
> assert in a debug build (you'd be keeping objects live at shutdown,=20
> leaking random stuff and possibly preventing finalizers from running.)
>=20
> During initialziation:
>=20
>  =C2=A0=C2=A0=C2=A0 rootedObjectsMap.init(cx);
>=20
> and before everything shuts down:
>=20
>  =C2=A0=C2=A0=C2=A0 rootedObjectsMap.reset();
>=20
> Using it would just be what you're used to:
>=20
>  =C2=A0 rooted.get().insert(ObjMap::value_type(std::string("foo"), obj));
>  =C2=A0 fprintf(stderr, "foo -> %p\n", (void*) rooted.get()[std::string("=
foo")]);
>=20
> Another option would be to use our GC-aware map API, GCHashMap. But=20
> again, you'd have to do template stuff because we don't natively know=20
> how to hash std::string, nor do we know that it's not a GC pointer type=
=20
> so it doesn't need to be traced or die during GC:
>=20
>  =C2=A0=C2=A0=C2=A0 namespace mozilla {
>  =C2=A0=C2=A0=C2=A0 template <>
>  =C2=A0=C2=A0=C2=A0 struct DefaultHasher<std::string> {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 using Key =3D std::string;
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 using Lookup =3D Key;
>=20
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 static HashNumber hash(const Lookup& aLoo=
kup) {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return HashString(aLookup.c_s=
tr());
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>=20
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 static bool match(const Key& aKey, const =
Lookup& aLookup) {
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return aKey =3D=3D aLookup;
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
>  =C2=A0=C2=A0=C2=A0 };
>  =C2=A0=C2=A0=C2=A0 } /* namespace mozilla */
>=20
>  =C2=A0=C2=A0=C2=A0 namespace JS {
>  =C2=A0=C2=A0=C2=A0 template <>
>  =C2=A0=C2=A0=C2=A0 struct GCPolicy<std::string> : public IgnoreGCPolicy<=
std::string> {};
>  =C2=A0=C2=A0=C2=A0 } /* namespace JS */
>=20
>  =C2=A0=C2=A0=C2=A0 using ObjMap =3D JS::GCHashMap<std::string, JS::Heap<=
JSObject*>,=20
> js::DefaultHasher<std::string>, js::SystemAllocPolicy>;
>=20
> The API is very different from std::map, though:
>=20
>  =C2=A0 rooted.put(std::string("foo"), obj);
>  =C2=A0 fprintf(stderr, "foo -> %p\n", (void*)=20
> rooted.lookup(std::string("foo"))->value());

0
tcampbell
4/12/2019 2:13:59 PM
Reply: