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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

msami
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::HandleObject value) {
    bool ok = 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 = 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 ‘std::pair<std::__cxx11::basic_string<char>, JS::Rooted<JSObject*> >::pair(const string&, JS::RootedObject&)’

 note:   types ‘std::tuple<_Args1 ...>’ and ‘const string {aka const std::__cxx11::basic_string<char>}’ have incompatible cv-qualifiers

 note:   types ‘std::pair<_T1, _T2>’ and ‘const string {aka const std::__cxx11::basic_string<char>}’ have incompatible cv-qualifiers

 note:   ‘const string {aka const std::__cxx11::basic_string<char>}’ is not derived from ‘const std::pair<_T1, _T2>’

 note:   candidate expects 0 arguments, 2 provided

 note:   ‘JS::Rooted<JSObject*>’ is not derived from ‘std::tuple<_Args1 ...>```
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

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

msami
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?
>
> typedef std::map <const std::string, JS::RootedObject> JSObjectsMap;
> JSObjectsMap rootedObjectsMap;
>
> ```
> bool MozillaJSEngine::addToRootedObjectsMap(const std::string& key, JS::HandleObject value) {
>     bool ok = 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 = 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 ‘std::pair<std::__cxx11::basic_string<char>, JS::Rooted<JSObject*> >::pair(const string&, JS::RootedObject&)’
>
>  note:   types ‘std::tuple<_Args1 ...>’ and ‘const string {aka const std::__cxx11::basic_string<char>}’ have incompatible cv-qualifiers
>
>  note:   types ‘std::pair<_T1, _T2>’ and ‘const string {aka const std::__cxx11::basic_string<char>}’ have incompatible cv-qualifiers
>
>  note:   ‘const string {aka const std::__cxx11::basic_string<char>}’ is not derived from ‘const std::pair<_T1, _T2>’
>
>  note:   candidate expects 0 arguments, 2 provided
>
>  note:   ‘JS::Rooted<JSObject*>’ is not derived from ‘std::tuple<_Args1 ...>```

so if I want to store JSObject in map and then set it on RootedObject will that cause problems on my application
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

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

Boris Zbarsky
In reply to this post by msami
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
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

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

Steve Fink-4
In reply to this post by msami
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.


_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

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

Steve Fink-4
In reply to this post by Boris Zbarsky
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());

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

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

tcampbell
I also offer up the examples here which try to explain this stuff. https://github.com/spidermonkey-embedders/spidermonkey-embedding-examples/blob/esr60/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.  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());

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine