- Revision
- 243672
- Author
- [email protected]
- Date
- 2019-03-29 18:30:16 -0700 (Fri, 29 Mar 2019)
Log Message
[JSC] JSWrapperMap should not use Objective-C Weak map (NSMapTable with NSPointerFunctionsWeakMemory) for m_cachedObjCWrappers
https://bugs.webkit.org/show_bug.cgi?id=196392
Reviewed by Saam Barati.
Weak representation in Objective-C is surprisingly costly in terms of memory. We can see that very easy program shows 10KB memory consumption due to
this weak wrapper map in _javascript_Core.framework. But we do not need this weak map since Objective-C JSValue has a dealloc. We can unregister itself
from the map when it is deallocated without using Objective-C weak mechanism. And since Objective-C JSValue is tightly coupled to a specific JSContext,
and wrapper map is created per JSContext, JSValue wrapper and actual _javascript_Core value is one-on-one, and [JSValue dealloc] knows which JSContext's
wrapper map holds itself.
1. We do not use Objective-C weak mechanism. We use WTF::HashSet instead. When JSValue is allocated, we register it to JSWrapperMap's HashSet. And unregister
JSValue from this map when JSValue is deallocated.
2. We use HashSet<JSValue> (logically) instead of HashMap<JSValueRef, JSValue> to keep JSValueRef and JSValue relationship. We can achieve it because JSValue
holds JSValueRef inside it.
* API/JSContext.mm:
(-[JSContext removeWrapper:]):
* API/JSContextInternal.h:
* API/JSValue.mm:
(-[JSValue dealloc]):
(-[JSValue initWithValue:inContext:]):
* API/JSWrapperMap.h:
* API/JSWrapperMap.mm:
(WrapperKey::hashTableDeletedValue):
(WrapperKey::WrapperKey):
(WrapperKey::isHashTableDeletedValue const):
(WrapperKey::Hash::hash):
(WrapperKey::Hash::equal):
(WrapperKey::Traits::isEmptyValue):
(WrapperKey::Translator::hash):
(WrapperKey::Translator::equal):
(WrapperKey::Translator::translate):
(-[JSWrapperMap initWithGlobalContextRef:]):
(-[JSWrapperMap dealloc]):
(-[JSWrapperMap objcWrapperForJSValueRef:inContext:]):
(-[JSWrapperMap removeWrapper:]):
* API/tests/testapi.mm:
(testObjectiveCAPIMain):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/JSContext.mm (243671 => 243672)
--- trunk/Source/_javascript_Core/API/JSContext.mm 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/API/JSContext.mm 2019-03-30 01:30:16 UTC (rev 243672)
@@ -369,6 +369,11 @@
return [[self wrapperMap] objcWrapperForJSValueRef:value inContext:self];
}
+- (void)removeWrapper:(JSValue *)value
+{
+ return [[self wrapperMap] removeWrapper:value];
+}
+
+ (JSContext *)contextWithJSGlobalContextRef:(JSGlobalContextRef)globalContext
{
JSContext *context = (__bridge JSContext *)toJSGlobalObject(globalContext)->apiWrapper();
Modified: trunk/Source/_javascript_Core/API/JSContextInternal.h (243671 => 243672)
--- trunk/Source/_javascript_Core/API/JSContextInternal.h 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/API/JSContextInternal.h 2019-03-30 01:30:16 UTC (rev 243672)
@@ -54,6 +54,7 @@
- (JSWrapperMap *)wrapperMap;
- (JSValue *)wrapperForObjCObject:(id)object;
- (JSValue *)wrapperForJSObject:(JSValueRef)value;
+- (void)removeWrapper:(JSValue *)value;
@end
Modified: trunk/Source/_javascript_Core/API/JSValue.mm (243671 => 243672)
--- trunk/Source/_javascript_Core/API/JSValue.mm 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/API/JSValue.mm 2019-03-30 01:30:16 UTC (rev 243672)
@@ -71,6 +71,7 @@
- (void)dealloc
{
+ [_context removeWrapper:self];
JSValueUnprotect([_context JSGlobalContextRef], m_value);
[_context release];
_context = nil;
@@ -1075,6 +1076,7 @@
if (!self)
return nil;
+ ASSERT(context);
_context = [context retain];
m_value = value;
JSValueProtect([_context JSGlobalContextRef], m_value);
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.h (243671 => 243672)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.h 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.h 2019-03-30 01:30:16 UTC (rev 243672)
@@ -37,6 +37,8 @@
- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:(JSContext *)context;
+- (void)removeWrapper:(JSValue *)wrapper;
+
@end
id tryUnwrapObjcObject(JSGlobalContextRef, JSValueRef);
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.mm (243671 => 243672)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2019-03-30 01:30:16 UTC (rev 243672)
@@ -581,10 +581,77 @@
@end
+struct WrapperKey {
+ static constexpr uintptr_t hashTableDeletedValue() { return 1; }
+
+ WrapperKey() = default;
+
+ explicit WrapperKey(WTF::HashTableDeletedValueType)
+ : m_wrapper(reinterpret_cast<JSValue *>(hashTableDeletedValue()))
+ {
+ }
+
+ explicit WrapperKey(JSValue *wrapper)
+ : m_wrapper(wrapper)
+ {
+ }
+
+ bool isHashTableDeletedValue() const
+ {
+ return reinterpret_cast<uintptr_t>(m_wrapper) == hashTableDeletedValue();
+ }
+
+ __unsafe_unretained JSValue *m_wrapper { nil };
+
+ struct Hash {
+ static unsigned hash(const WrapperKey& key)
+ {
+ return DefaultHash<JSValueRef>::Hash::hash([key.m_wrapper JSValueRef]);
+ }
+
+ static bool equal(const WrapperKey& lhs, const WrapperKey& rhs)
+ {
+ return lhs.m_wrapper == rhs.m_wrapper;
+ }
+
+ static const bool safeToCompareToEmptyOrDeleted = false;
+ };
+
+ struct Traits : public SimpleClassHashTraits<WrapperKey> {
+ static const bool hasIsEmptyValueFunction = true;
+ static bool isEmptyValue(const WrapperKey& key)
+ {
+ return key.m_wrapper == nullptr;
+ }
+ };
+
+ struct Translator {
+ struct ValueAndContext {
+ __unsafe_unretained JSContext *m_context;
+ JSValueRef m_value;
+ };
+
+ static unsigned hash(const ValueAndContext& value)
+ {
+ return DefaultHash<JSValueRef>::Hash::hash(value.m_value);
+ }
+
+ static bool equal(const WrapperKey& lhs, const ValueAndContext& value)
+ {
+ return [lhs.m_wrapper JSValueRef] == value.m_value;
+ }
+
+ static void translate(WrapperKey& result, const ValueAndContext& value, unsigned)
+ {
+ result = WrapperKey([[[JSValue alloc] initWithValue:value.m_value inContext:value.m_context] autorelease]);
+ }
+ };
+};
+
@implementation JSWrapperMap {
NSMutableDictionary *m_classMap;
std::unique_ptr<JSC::WeakGCMap<__unsafe_unretained id, JSC::JSObject>> m_cachedJSWrappers;
- NSMapTable *m_cachedObjCWrappers;
+ HashSet<WrapperKey, WrapperKey::Hash, WrapperKey::Traits> m_cachedObjCWrappers;
}
- (instancetype)initWithGlobalContextRef:(JSGlobalContextRef)context
@@ -593,10 +660,6 @@
if (!self)
return nil;
- NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
- NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
- m_cachedObjCWrappers = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
-
m_cachedJSWrappers = std::make_unique<JSC::WeakGCMap<__unsafe_unretained id, JSC::JSObject>>(toJS(context)->vm());
ASSERT(!toJSGlobalObject(context)->wrapperMap());
@@ -607,7 +670,6 @@
- (void)dealloc
{
- [m_cachedObjCWrappers release];
[m_classMap release];
[super dealloc];
}
@@ -662,14 +724,16 @@
- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:context
{
ASSERT(toJSGlobalObject([context JSGlobalContextRef])->wrapperMap() == self);
- JSValue *wrapper = (__bridge JSValue *)NSMapGet(m_cachedObjCWrappers, value);
- if (!wrapper) {
- wrapper = [[[JSValue alloc] initWithValue:value inContext:context] autorelease];
- NSMapInsert(m_cachedObjCWrappers, value, (__bridge void*)wrapper);
- }
- return wrapper;
+ WrapperKey::Translator::ValueAndContext valueAndContext { context, value };
+ auto addResult = m_cachedObjCWrappers.add<WrapperKey::Translator>(valueAndContext);
+ return addResult.iterator->m_wrapper;
}
+- (void)removeWrapper:(JSValue *)wrapper
+{
+ m_cachedObjCWrappers.remove(WrapperKey(wrapper));
+}
+
@end
id tryUnwrapObjcObject(JSGlobalContextRef context, JSValueRef value)
Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (243671 => 243672)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2019-03-30 01:30:16 UTC (rev 243672)
@@ -565,12 +565,28 @@
static void testObjectiveCAPIMain()
{
@autoreleasepool {
- JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
- JSContext* context = [[JSContext alloc] initWithVirtualMachine:vm];
+ JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
+ JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
[context evaluateScript:@"bad"];
}
@autoreleasepool {
+ JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
+ JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+ JSValue *number1 = [context evaluateScript:@"42092389"];
+ JSValue *number2 = [context evaluateScript:@"42092389"];
+ checkResult(@"wrapper cache for numbers", number1 == number2 && number1.isNumber && [number1 toInt32] == 42092389);
+ }
+
+ @autoreleasepool {
+ JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
+ JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+ JSValue *object1 = [context evaluateScript:@"({})"];
+ JSValue *object2 = [context evaluateScript:@"({})"];
+ checkResult(@"wrapper cache for objects", object1 != object2);
+ }
+
+ @autoreleasepool {
JSContext *context = [[JSContext alloc] init];
JSValue *result = [context evaluateScript:@"2 + 2"];
checkResult(@"2 + 2", result.isNumber && [result toInt32] == 4);
Modified: trunk/Source/_javascript_Core/ChangeLog (243671 => 243672)
--- trunk/Source/_javascript_Core/ChangeLog 2019-03-30 01:09:12 UTC (rev 243671)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-03-30 01:30:16 UTC (rev 243672)
@@ -1,3 +1,45 @@
+2019-03-29 Yusuke Suzuki <[email protected]>
+
+ [JSC] JSWrapperMap should not use Objective-C Weak map (NSMapTable with NSPointerFunctionsWeakMemory) for m_cachedObjCWrappers
+ https://bugs.webkit.org/show_bug.cgi?id=196392
+
+ Reviewed by Saam Barati.
+
+ Weak representation in Objective-C is surprisingly costly in terms of memory. We can see that very easy program shows 10KB memory consumption due to
+ this weak wrapper map in _javascript_Core.framework. But we do not need this weak map since Objective-C JSValue has a dealloc. We can unregister itself
+ from the map when it is deallocated without using Objective-C weak mechanism. And since Objective-C JSValue is tightly coupled to a specific JSContext,
+ and wrapper map is created per JSContext, JSValue wrapper and actual _javascript_Core value is one-on-one, and [JSValue dealloc] knows which JSContext's
+ wrapper map holds itself.
+
+ 1. We do not use Objective-C weak mechanism. We use WTF::HashSet instead. When JSValue is allocated, we register it to JSWrapperMap's HashSet. And unregister
+ JSValue from this map when JSValue is deallocated.
+ 2. We use HashSet<JSValue> (logically) instead of HashMap<JSValueRef, JSValue> to keep JSValueRef and JSValue relationship. We can achieve it because JSValue
+ holds JSValueRef inside it.
+
+ * API/JSContext.mm:
+ (-[JSContext removeWrapper:]):
+ * API/JSContextInternal.h:
+ * API/JSValue.mm:
+ (-[JSValue dealloc]):
+ (-[JSValue initWithValue:inContext:]):
+ * API/JSWrapperMap.h:
+ * API/JSWrapperMap.mm:
+ (WrapperKey::hashTableDeletedValue):
+ (WrapperKey::WrapperKey):
+ (WrapperKey::isHashTableDeletedValue const):
+ (WrapperKey::Hash::hash):
+ (WrapperKey::Hash::equal):
+ (WrapperKey::Traits::isEmptyValue):
+ (WrapperKey::Translator::hash):
+ (WrapperKey::Translator::equal):
+ (WrapperKey::Translator::translate):
+ (-[JSWrapperMap initWithGlobalContextRef:]):
+ (-[JSWrapperMap dealloc]):
+ (-[JSWrapperMap objcWrapperForJSValueRef:inContext:]):
+ (-[JSWrapperMap removeWrapper:]):
+ * API/tests/testapi.mm:
+ (testObjectiveCAPIMain):
+
2019-03-29 Robin Morisset <[email protected]>
B3ReduceStrength should know that Mul distributes over Add and Sub
