1
0
forked from 0ad/0ad

Use Symbols to store JS object references when serialising and delete ObjectIDCache

When serialising JS objects, we keep track of any encountered object,
and serialize it only once. Any further serialisation instead stores an
ID referring to the original object (essentially an opaque pointer).
The trouble of course is to have a unique, persistent identifier for
such an object.
svn uses an ObjectIDCache, essentially a "JS Object -> ID" map (which
internally is essentially a "JS heap pointer -> ID" map).

JS, since ES15, includes a "Symbol" primitive type, which is a unique,
immutable identifier. They are also not iterable by for..in or
GetOwnPropertyName or related.
This means they can be used to store the tag directly on the object
(since it's impossible overwrite a user property).
Thanks to this, we can forgo ObjectIDCache in the serializers, and since
following D2897 it becomes unused, we can delete it, along with the
Finalization code it used.


Part of SM52 migration, stage: SM45-compatible changes.

Patch by: Itms
Tested By: Freagarach
Refs #4893

Differential Revision: https://code.wildfiregames.com/D3085
This was SVN commit r24167.
This commit is contained in:
wraitii 2020-11-12 06:40:19 +00:00
parent d5bbf900bc
commit dad2857538
12 changed files with 52 additions and 278 deletions

View File

@ -53,7 +53,7 @@ SODIUM_VERSION="libsodium-1.0.18"
# * FCollada
# --------------------------------------------------------------
# We use suffixes here in order to force rebuilding when patching these libs
SPIDERMONKEY_VERSION="mozjs-45.0.2+wildfiregames.1"
SPIDERMONKEY_VERSION="mozjs-45.0.2+wildfiregames.2"
NVTT_VERSION="nvtt-2.1.1+wildfiregames.1"
FCOLLADA_VERSION="fcollada-3.05+wildfiregames.1"
# --------------------------------------------------------------

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2016 Wildfire Games.
/* Copyright (C) 2020 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -89,22 +89,6 @@ void GCSliceCallbackHook(JSRuntime* UNUSED(rt), JS::GCProgress progress, const J
#endif
}
void ScriptRuntime::GCCallback(JSRuntime* UNUSED(rt), JSGCStatus status, void *data)
{
if (status == JSGC_END)
reinterpret_cast<ScriptRuntime*>(data)->GCCallbackMember();
}
void ScriptRuntime::GCCallbackMember()
{
m_FinalizationListObjectIdCache.clear();
}
void ScriptRuntime::AddDeferredFinalizationObject(const std::shared_ptr<void>& obj)
{
m_FinalizationListObjectIdCache.push_back(obj);
}
ScriptRuntime::ScriptRuntime(shared_ptr<ScriptRuntime> parentRuntime, int runtimeSize, int heapGrowthBytesGCTrigger):
m_LastGCBytes(0),
m_LastGCCheck(0.0f),
@ -118,7 +102,6 @@ ScriptRuntime::ScriptRuntime(shared_ptr<ScriptRuntime> parentRuntime, int runtim
ENSURE(m_rt); // TODO: error handling
JS::SetGCSliceCallback(m_rt, GCSliceCallbackHook);
JS_SetGCCallback(m_rt, ScriptRuntime::GCCallback, this);
JS_SetGCParameter(m_rt, JSGC_MAX_MALLOC_BYTES, m_RuntimeSize);
JS_SetGCParameter(m_rt, JSGC_MAX_BYTES, m_RuntimeSize);
@ -133,9 +116,7 @@ ScriptRuntime::ScriptRuntime(shared_ptr<ScriptRuntime> parentRuntime, int runtim
ScriptRuntime::~ScriptRuntime()
{
JS_SetGCCallback(m_rt, nullptr, nullptr);
JS_DestroyRuntime(m_rt);
ENSURE(m_FinalizationListObjectIdCache.empty() && "Leak: Removing callback while some objects still aren't finalized!");
ENSURE(ScriptEngine::IsInitialised() && "The ScriptEngine must be active (initialized and not yet shut down) when destroying a ScriptRuntime!");
ScriptEngine::GetSingleton().UnRegisterRuntime(m_rt);

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2018 Wildfire Games.
/* Copyright (C) 2020 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -57,32 +57,18 @@ public:
void RegisterContext(JSContext* cx);
void UnRegisterContext(JSContext* cx);
/**
* Registers an object to be freed/finalized by the ScriptRuntime. Freeing is
* guaranteed to happen after the next minor GC has completed, but might also
* happen a bit later. This is only needed in very special situations
* and you should only use it if you know exactly why you need it!
* Specify a deleter for the shared_ptr to free the void pointer correctly
* (by casting to the right type before calling delete for example).
*/
void AddDeferredFinalizationObject(const std::shared_ptr<void>& obj);
JSRuntime* m_rt;
private:
void PrepareContextsForIncrementalGC();
void GCCallbackMember();
std::list<JSContext*> m_Contexts;
std::vector<std::shared_ptr<void> > m_FinalizationListObjectIdCache;
int m_RuntimeSize;
int m_HeapGrowthBytesGCTrigger;
int m_LastGCBytes;
double m_LastGCCheck;
static void GCCallback(JSRuntime *rt, JSGCStatus status, void *data);
};
#endif // INCLUDED_SCRIPTRUNTIME

View File

@ -1,64 +0,0 @@
/* Copyright (C) 2016 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2 of the License, or
* (at your option) any later version.
*
* 0 A.D. is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with 0 A.D. If not, see <http://www.gnu.org/licenses/>.
*/
#include "lib/self_test.h"
#include "scriptinterface/ScriptInterface.h"
#include "scriptinterface/ScriptRuntime.h"
#include "scriptinterface/third_party/ObjectToIDMap.h"
class TestObjectToIDMap : public CxxTest::TestSuite
{
public:
void test_movinggc()
{
ScriptInterface script("Test", "Test", g_ScriptRuntime);
JSContext* cx = script.GetContext();
JSAutoRequest rq(cx);
JS::RootedObject obj(cx, JS_NewPlainObject(cx));
ObjectIdCache<u32> map(g_ScriptRuntime);
map.init();
TS_ASSERT(map.add(cx, obj, 1));
JSObject* plainObj = obj;
// The map should contain the object we've just added
TS_ASSERT(map.has(plainObj));
JS_GC(g_ScriptRuntime->m_rt);
// After a GC, the object should have been moved and plainObj should
// not be valid anymore and not be found in the map anymore.
// Obj should have an updated reference too, so it should still be found
// in the map.
//
// NOTE: It's observed behaviour that a full GC always moves an object.
// This might change in future SpiderMonkey versions. We only rely on
// that behaviour for this test.
//
// TODO: It might be a good idea to test the behaviour when only a minor
// GC runs, but there's no API for calling a minor GC yet.
TS_ASSERT(plainObj != obj);
TS_ASSERT(!map.has(plainObj));
TS_ASSERT(map.has(obj));
// Finding the ID associated with the object
u32 ret(0);
TS_ASSERT(map.find(obj, ret));
TS_ASSERT_EQUALS(ret, 1);
}
};

View File

@ -1,132 +0,0 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
/**
* Providing a map-like structure with JSObject pointers (actually their hash) as keys
* with correct garbage collection handling (JSObjects can move in memory).
*
* The code in this class was copied from here and modified to work in our environment.
* * https://mxr.mozilla.org/mozilla-esr38/source/js/ipc/JavaScriptShared.h
* * https://mxr.mozilla.org/mozilla-esr38/source/js/ipc/JavaScriptShared.cpp
*
* When updating SpiderMonkey, you most likely have to reintegrate an updated version
* of the class(es) in this file. The best way is probably to get a diff between the
* original files and integrate that because this file is heavily modified from the
* original version.
*/
#ifndef INCLUDED_OBJECTTOIDMAP
#define INCLUDED_OBJECTTOIDMAP
#include "scriptinterface/ScriptRuntime.h"
#include "scriptinterface/ScriptTypes.h"
#include <stdint.h>
// Map JSObjects -> ids
template <typename T>
class ObjectIdCache
{
typedef js::PointerHasher<JSObject*, 3> Hasher;
typedef js::HashMap<JSObject*, T, Hasher, js::SystemAllocPolicy> Table;
NONCOPYABLE(ObjectIdCache);
public:
ObjectIdCache(shared_ptr<ScriptRuntime> rt)
: table_(nullptr), m_rt(rt)
{
}
~ObjectIdCache()
{
if (table_)
{
m_rt->AddDeferredFinalizationObject(std::shared_ptr<void>((void*)table_, DeleteTable));
table_ = nullptr;
JS_RemoveExtraGCRootsTracer(m_rt->m_rt, ObjectIdCache::Trace, this);
}
}
bool init()
{
if (table_)
return true;
table_ = new Table(js::SystemAllocPolicy());
JS_AddExtraGCRootsTracer(m_rt->m_rt, ObjectIdCache::Trace, this);
return table_ && table_->init(32);
}
void trace(JSTracer* trc)
{
for (typename Table::Enum e(*table_); !e.empty(); e.popFront())
{
JSObject* obj = e.front().key();
JS_CallUnbarrieredObjectTracer(trc, &obj, "ipc-object");
if (obj != e.front().key())
e.rekeyFront(obj);
}
}
// TODO sweep?
bool find(JSObject* obj, T& ret)
{
typename Table::Ptr p = table_->lookup(obj);
if (!p)
return false;
ret = p->value();
return true;
}
bool add(JSContext* cx, JSObject* obj, T id)
{
if (!table_->put(obj, id))
return false;
JS_StoreObjectPostBarrierCallback(cx, keyMarkCallback, obj, table_);
return true;
}
void remove(JSObject* obj)
{
table_->remove(obj);
}
// TODO clear?
bool empty()
{
return table_->empty();
}
bool has(JSObject* obj)
{
return table_->has(obj);
}
private:
static void keyMarkCallback(JSTracer* trc, JSObject* key, void* data)
{
Table* table = static_cast<Table*>(data);
JSObject* prior = key;
JS_CallUnbarrieredObjectTracer(trc, &key, "ObjectIdCache::table_ key");
table->rekeyIfMoved(prior, key);
}
static void Trace(JSTracer* trc, void* data)
{
reinterpret_cast<ObjectIdCache*>(data)->trace(trc);
}
static void DeleteTable(void* table)
{
delete (Table*)table;
}
shared_ptr<ScriptRuntime> m_rt;
Table* table_;
};
#endif // INCLUDED_OBJECTTOIDMAP

View File

@ -32,6 +32,7 @@
#include "ps/scripting/JSInterface_VFS.h"
#include "ps/TemplateLoader.h"
#include "ps/Util.h"
#include "scriptinterface/ScriptRuntime.h"
#include "simulation2/components/ICmpAIInterface.h"
#include "simulation2/components/ICmpCommandQueue.h"
#include "simulation2/components/ICmpObstructionManager.h"

View File

@ -1,4 +1,4 @@
/* Copyright (C) 2015 Wildfire Games.
/* Copyright (C) 2020 Wildfire Games.
* This file is part of 0 A.D.
*
* 0 A.D. is free software: you can redistribute it and/or modify
@ -49,7 +49,7 @@ public:
TS_ASSERT(test.GetScriptInterface().Eval("([1,2,3])", &cmd));
cmp->PushLocalCommand(1, cmd);
TS_ASSERT(test.GetScriptInterface().Eval("({x:4})", &cmd));
TS_ASSERT(test.GetScriptInterface().Eval("({\"x\":4})", &cmd));
cmp->PushLocalCommand(-1, cmd);
test.Roundtrip();
@ -57,7 +57,7 @@ public:
// Process the first two commands
cmp->FlushTurn(empty);
TS_ASSERT(test.GetScriptInterface().Eval("({y:5})", &cmd));
TS_ASSERT(test.GetScriptInterface().Eval("({\"y\":5})", &cmd));
cmp->PushLocalCommand(10, cmd);
// Process the next command
@ -69,7 +69,7 @@ public:
test.Roundtrip();
std::string output;
TS_ASSERT(test.GetScriptInterface().Eval("uneval(cmds)", output));
TS_ASSERT_STR_EQUALS(output, "[[1, [1, 2, 3]], [-1, {x:4}], [10, {y:5}]]");
TS_ASSERT(test.GetScriptInterface().Eval("JSON.stringify(cmds)", output));
TS_ASSERT_STR_EQUALS(output, "[[1,[1,2,3]],[-1,{\"x\":4}],[10,{\"y\":5}]]");
}
};

View File

@ -55,10 +55,12 @@ static u8 GetArrayType(js::Scalar::Type arrayType)
}
CBinarySerializerScriptImpl::CBinarySerializerScriptImpl(const ScriptInterface& scriptInterface, ISerializer& serializer) :
m_ScriptInterface(scriptInterface), m_Serializer(serializer), m_ScriptBackrefs(scriptInterface.GetRuntime()),
m_ScriptBackrefsNext(1)
m_ScriptInterface(scriptInterface), m_Serializer(serializer), m_ScriptBackrefsNext(0)
{
m_ScriptBackrefs.init();
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
m_ScriptBackrefSymbol.init(cx, JS::NewSymbol(cx, nullptr));
}
void CBinarySerializerScriptImpl::HandleScriptVal(JS::HandleValue val)
@ -89,11 +91,11 @@ void CBinarySerializerScriptImpl::HandleScriptVal(JS::HandleValue val)
JS::RootedObject obj(cx, &val.toObject());
// If we've already serialized this object, just output a reference to it
u32 tag = GetScriptBackrefTag(obj);
if (tag)
i32 tag = GetScriptBackrefTag(obj);
if (tag != -1)
{
m_Serializer.NumberU8_Unbounded("type", SCRIPT_TYPE_BACKREF);
m_Serializer.NumberU32_Unbounded("tag", tag);
m_Serializer.NumberI32("tag", tag, 0, JSVAL_INT_MAX);
break;
}
@ -406,27 +408,39 @@ void CBinarySerializerScriptImpl::ScriptString(const char* name, JS::HandleStrin
}
}
u32 CBinarySerializerScriptImpl::GetScriptBackrefTag(JS::HandleObject obj)
i32 CBinarySerializerScriptImpl::GetScriptBackrefTag(JS::HandleObject obj)
{
// To support non-tree structures (e.g. "var x = []; var y = [x, x];"), we need a way
// to indicate multiple references to one object(/array). So every time we serialize a
// new object, we give it a new non-zero tag; when we serialize it a second time we just
// refer to that tag.
// new object, we give it a new tag; when we serialize it a second time we just refer
// to that tag.
//
// The tags are stored in a map. Maybe it'd be more efficient to store it inline in the object
// somehow? but this works okay for now
// If it was already there, return the tag
u32 tag;
if (m_ScriptBackrefs.find(obj, tag))
return tag;
// Tags are stored on the object. To avoid overwriting any existing property,
// they are saved as a uniquely-named, non-enumerable property (the serializer's unique symbol).
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
m_ScriptBackrefs.add(cx, obj, m_ScriptBackrefsNext);
JS::RootedValue symbolValue(cx, JS::SymbolValue(m_ScriptBackrefSymbol));
JS::RootedId symbolId(cx);
ENSURE(JS_ValueToId(cx, symbolValue, &symbolId));
m_ScriptBackrefsNext++;
JS::RootedValue tagValue(cx);
// If it was already there, return the tag
bool tagFound;
ENSURE(JS_HasPropertyById(cx, obj, symbolId, &tagFound));
if (tagFound)
{
ENSURE(JS_GetPropertyById(cx, obj, symbolId, &tagValue));
ENSURE(tagValue.isInt32());
return tagValue.toInt32();
}
tagValue = JS::Int32Value(m_ScriptBackrefsNext);
JS_SetPropertyById(cx, obj, symbolId, tagValue);
++m_ScriptBackrefsNext;
// Return a non-tag number so callers know they need to serialize the object
return 0;
return -1;
}

View File

@ -20,8 +20,6 @@
#include "ISerializer.h"
#include "scriptinterface/third_party/ObjectToIDMap.h"
#include "lib/byte_order.h"
#include "lib/allocators/arena.h"
@ -91,9 +89,9 @@ private:
const ScriptInterface& m_ScriptInterface;
ISerializer& m_Serializer;
ObjectIdCache<u32> m_ScriptBackrefs;
u32 m_ScriptBackrefsNext;
u32 GetScriptBackrefTag(JS::HandleObject obj);
JS::PersistentRootedSymbol m_ScriptBackrefSymbol;
i32 m_ScriptBackrefsNext;
i32 GetScriptBackrefTag(JS::HandleObject obj);
};
/**

View File

@ -28,18 +28,9 @@
#include "lib/byte_order.h"
CStdDeserializer::CStdDeserializer(const ScriptInterface& scriptInterface, std::istream& stream) :
m_ScriptInterface(scriptInterface), m_Stream(stream),
m_dummyObject(scriptInterface.GetJSRuntime())
m_ScriptInterface(scriptInterface), m_Stream(stream)
{
JSContext* cx = m_ScriptInterface.GetContext();
JSAutoRequest rq(cx);
JS_AddExtraGCRootsTracer(m_ScriptInterface.GetJSRuntime(), CStdDeserializer::Trace, this);
// Add a dummy tag because the serializer uses the tag 0 to indicate that a value
// needs to be serialized and then tagged
m_dummyObject = JS_NewPlainObject(cx);
m_ScriptBackrefs.push_back(JS::Heap<JSObject*>(m_dummyObject));
}
CStdDeserializer::~CStdDeserializer()
@ -111,7 +102,7 @@ void CStdDeserializer::AddScriptBackref(JS::HandleObject obj)
m_ScriptBackrefs.push_back(JS::Heap<JSObject*>(obj));
}
void CStdDeserializer::GetScriptBackref(u32 tag, JS::MutableHandleObject ret)
void CStdDeserializer::GetScriptBackref(size_t tag, JS::MutableHandleObject ret)
{
ENSURE(m_ScriptBackrefs.size() > tag);
ret.set(m_ScriptBackrefs[tag]);
@ -217,8 +208,8 @@ JS::Value CStdDeserializer::ReadScriptVal(const char* UNUSED(name), JS::HandleOb
}
case SCRIPT_TYPE_BACKREF:
{
u32 tag;
NumberU32_Unbounded("tag", tag);
i32 tag;
NumberI32("tag", tag, 0, JSVAL_INT_MAX);
JS::RootedObject obj(cx);
GetScriptBackref(tag, &obj);
if (!obj)

View File

@ -51,9 +51,8 @@ private:
void ReadStringUTF16(const char* name, utf16string& str);
virtual void AddScriptBackref(JS::HandleObject obj);
virtual void GetScriptBackref(u32 tag, JS::MutableHandleObject ret);
virtual void GetScriptBackref(size_t tag, JS::MutableHandleObject ret);
std::vector<JS::Heap<JSObject*> > m_ScriptBackrefs;
JS::PersistentRooted<JSObject*> m_dummyObject;
const ScriptInterface& m_ScriptInterface;

View File

@ -689,7 +689,7 @@ public:
"\x05" // SCRIPT_TYPE_INT
"\x02\0\0\0" // 2
"\x08" // SCRIPT_TYPE_BACKREF
"\x02\0\0\0" // ref. to object #2, i.e. "b", with #1 being "a"
"\x01\0\0\0" // ref. to object #1, i.e. "b", with #0 being "a"
);
}
@ -711,7 +711,7 @@ public:
"\x01\0\0\0" // num props
"\x01\x03\0\0\0" "bar" // "bar"
"\x08" // SCRIPT_TYPE_BACKREF
"\x02\0\0\0" // ref to object #2, i.e. "b", with #1 being "a"
"\x01\0\0\0" // ref to object #1, i.e. "b", with #0 being "a"
);
}