Skip to content

swb: annotate BaseDictionary entry items #1819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 26, 2016
Merged

swb: annotate BaseDictionary entry items #1819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
747216b
swb: annotate BaseDictionary entry items
Oct 22, 2016
3a170fb
swb: simplify write barrier macros
Oct 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions lib/Common/DataStructures/BaseDictionary.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ namespace JsUtil
typedef TValue ValueType;
typedef typename AllocatorInfo<TAllocator, TValue>::AllocatorType AllocatorType;
typedef SizePolicy CurrentSizePolicy;
typedef Entry<TKey, TValue> EntryType;
typedef Entry<
Field(TKey, TAllocator),
Field(TValue, TAllocator)> EntryType;

template<class TDictionary> class EntryIterator;
template<class TDictionary> class BucketEntryIterator;
Expand All @@ -90,17 +92,17 @@ namespace JsUtil
friend class Js::RemoteDictionary<BaseDictionary>;
template <typename ValueOrKey> struct ComparerType { typedef Comparer<ValueOrKey> Type; }; // Used by diagnostics to access Comparer type

Pointer(int, TAllocator) buckets;
Pointer(EntryType, TAllocator) entries;
PointerNoBarrier(AllocatorType) alloc;
Field(int*, TAllocator) buckets;
Field(EntryType*, TAllocator) entries;
FieldNoBarrier(AllocatorType*) alloc;
Field(int) size;
Field(uint) bucketCount;
Field(int) count;
Field(int) freeList;
Field(int) freeCount;

#if PROFILE_DICTIONARY
PointerNoBarrier(DictionaryStats) stats;
FieldNoBarrier(DictionaryStats*) stats;
#endif
enum InsertOperations
{
Expand Down Expand Up @@ -370,7 +372,8 @@ namespace JsUtil
template <typename TLookup>
bool TryGetReference(const TLookup& key, TValue** value) const
{
return TryGetReference(key, const_cast<const TValue **>(value));
int i;
return TryGetReference(key, value, &i);
}

template <typename TLookup>
Expand All @@ -389,7 +392,14 @@ namespace JsUtil
template <typename TLookup>
bool TryGetReference(const TLookup& key, TValue** value, int* index) const
{
return TryGetReference(key, const_cast<const TValue **>(value), index);
int i = FindEntryWithKey(key);
if (i >= 0)
{
*value = &entries[i].Value();
*index = i;
return true;
}
return false;
}

const TValue& GetValueAt(const int index) const
Expand Down Expand Up @@ -1385,7 +1395,7 @@ namespace JsUtil
class BaseHashSet : protected BaseDictionary<TKey, TElement, TAllocator, SizePolicy, Comparer, Entry, Lock>
{
typedef BaseDictionary<TKey, TElement, TAllocator, SizePolicy, Comparer, Entry, Lock> Base;
typedef Entry<TKey, TElement> EntryType;
typedef typename Base::EntryType EntryType;
typedef typename Base::AllocatorType AllocatorType;
friend struct JsDiag::RemoteDictionary<BaseHashSet<TElement, TAllocator, SizePolicy, TKey, Comparer, Entry, Lock>>;

Expand Down Expand Up @@ -1559,7 +1569,7 @@ namespace JsUtil
class SynchronizedDictionary: protected BaseDictionary<TKey, TValue, TAllocator, SizePolicy, Comparer, Entry>
{
private:
PointerNoBarrier(SyncObject) syncObj;
FieldNoBarrier(SyncObject*) syncObj;

typedef BaseDictionary<TKey, TValue, TAllocator, SizePolicy, Comparer, Entry> Base;
public:
Expand Down
2 changes: 1 addition & 1 deletion lib/Common/DataStructures/Cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ namespace JsUtil
return cacheStore->TryGetValue(key, value);
}

bool TryGetReference(const TKey& key, TValue** value, int* index)
bool TryGetReference(const TKey& key, const TValue** value, int* index)
{
return cacheStore->TryGetReference(key, value, index);
}
Expand Down
37 changes: 19 additions & 18 deletions lib/Common/Memory/RecyclerPointers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,17 @@ struct AllocatorWriteBarrierPolicy<RecyclerNonLeafAllocator, T> { typedef _write
template <>
struct AllocatorWriteBarrierPolicy<RecyclerNonLeafAllocator, int> { typedef _no_write_barrier_policy Policy; };

// Choose WriteBarrierPtr or NoWriteBarrierPtr based on Policy
// Choose write barrier Field type: T unchanged, or WriteBarrierPtr based on Policy.
//
template <class T, class Policy>
struct _WriteBarrierPtrPolicy { typedef NoWriteBarrierPtr<T> Ptr; };
struct _WriteBarrierFieldType { typedef T Type; };
template <class T>
struct _WriteBarrierPtrPolicy<T, _write_barrier_policy> { typedef WriteBarrierPtr<T> Ptr; };
struct _WriteBarrierFieldType<T*, _write_barrier_policy> { typedef WriteBarrierPtr<T> Type; };

// Choose WriteBarrierPtr or NoWriteBarrierPtr based on Allocator and T* type
//
template <class T,
class Allocator = Recycler,
class Policy = typename AllocatorWriteBarrierPolicy<Allocator, T*>::Policy>
struct WriteBarrierPtrTraits { typedef typename _WriteBarrierPtrPolicy<T, Policy>::Ptr Ptr; };
class Policy = typename AllocatorWriteBarrierPolicy<Allocator, T>::Policy>
struct WriteBarrierFieldTypeTraits { typedef typename _WriteBarrierFieldType<T, Policy>::Type Type; };

// ArrayWriteBarrier behavior
//
Expand Down Expand Up @@ -163,9 +161,9 @@ class NoWriteBarrierPtr

// Getters
T * operator->() const { return this->value; }
operator T*() const { return this->value; }
operator T* const & () const { return this->value; }

T const** operator&() const { return &value; }
T* const * operator&() const { return &value; }
T** operator&() { return &value; }

// Setters
Expand Down Expand Up @@ -216,21 +214,24 @@ class WriteBarrierPtr

// Getters
T * operator->() const { return ptr; }
operator T*() const { return ptr; }
operator T* const & () const { return ptr; }

T const** AddressOf() const { return &ptr; }
T* const * AddressOf() const { return &ptr; }
T** AddressOf() { return &ptr; }

T const** operator&() const
{
static_assert(false, "Might need to set barrier for this operation, and use AddressOf instead.");
return &ptr;
}
T** operator&()
// Taking immutable address is ok
Copy link
Contributor

@leirocks leirocks Oct 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking immutable address is ok [](start = 7, length = 30)

we might need const_cast check in our tool? if so please add a TODO here

Copy link
Author

@jianchun jianchun Oct 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked offline. @leirocks's idea is to examine and baseline our usage of const_cast to prevent future incorrect use related to this code. That is doable. Will add a note here.

//
T* const * operator&() const
{
static_assert(false, "Might need to set barrier for this operation, and use AddressOf instead.");
return &ptr;
}
// Taking mutable address is not allowed
//
// T** operator&()
// {
// static_assert(false, "Might need to set barrier for this operation, and use AddressOf instead.");
// return &ptr;
// }

// Setters
WriteBarrierPtr& operator=(T * ptr)
Expand Down
21 changes: 9 additions & 12 deletions lib/Common/Memory/WriteBarrierMacros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,33 +28,30 @@

#define SAVE_WRITE_BARRIER_MACROS() \
PushMacro("Field") \
PushMacro("PointerNoBarrier") \
PushMacro("Pointer")
PushMacro("FieldNoBarrier")

#define RESTORE_WRITE_BARRIER_MACROS() \
PopMacro("Field") \
PopMacro("PointerNoBarrier") \
PopMacro("Pointer")
PopMacro("FieldNoBarrier")

#endif

#ifdef FORCE_USE_WRITE_BARRIER
SAVE_WRITE_BARRIER_MACROS()
#undef Field
#undef PointerNoBarrier
#undef Pointer
#undef FieldNoBarrier
#endif

// TODO: Turn off these annotations on Win32
#if defined(__clang__) || defined(FORCE_USE_WRITE_BARRIER)
// Various macros for defining field attributes
#define Field(type) NoWriteBarrierField<type>
#define PointerNoBarrier(type) NoWriteBarrierPtr<type>
#define Pointer(type, ...) typename WriteBarrierPtrTraits<type, ##__VA_ARGS__>::Ptr
#define Field(type, ...) \
typename WriteBarrierFieldTypeTraits<type, ##__VA_ARGS__>::Type
#define FieldNoBarrier(type) \
typename WriteBarrierFieldTypeTraits<type, _no_write_barrier_policy, _no_write_barrier_policy>::Type
#else
#define Field(type) type
#define PointerNoBarrier(type) type*
#define Pointer(type, ...) type*
#define Field(type, ...) type
#define FieldNoBarrier(type) type
#endif

#undef FORCE_USE_WRITE_BARRIER
Expand Down
4 changes: 2 additions & 2 deletions lib/Runtime/Base/AuxPtrs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace Js
static const uint8 MaxCount;
Field(uint8) count; // always saving maxCount
Field(FieldsEnum) type[_MaxCount]; // save instantiated pointer enum
Pointer(void) ptr[_MaxCount]; // save instantiated pointer address
Field(void*) ptr[_MaxCount]; // save instantiated pointer address
AuxPtrsFix();
AuxPtrsFix(AuxPtrsFix<FieldsEnum, 16>* ptr16); // called when promoting from AuxPtrs16 to AuxPtrs32
void* Get(FieldsEnum e);
Expand All @@ -53,7 +53,7 @@ namespace Js
Field(uint8) count; // save instantiated pointers count
Field(uint8) capacity; // save number of pointers can be hold in current instance of AuxPtrs
Field(uint8) offsets[static_cast<int>(FieldsEnum::Max)]; // save position of each instantiated pointers, if not instantiate, it's invalid
Pointer(void) ptrs[1]; // instantiated pointer addresses
Field(void*) ptrs[1]; // instantiated pointer addresses
AuxPtrs(uint8 capacity, AuxPtrs32* ptr32); // called when promoting from AuxPtrs32 to AuxPtrs
AuxPtrs(uint8 capacity, AuxPtrs* ptr); // called when expanding (i.e. promoting from AuxPtrs to bigger AuxPtrs)
void* Get(FieldsEnum e);
Expand Down
2 changes: 1 addition & 1 deletion lib/Runtime/Base/CompactCounters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace Js
mutable Field(bool) bgThreadCallStarted;
Field(bool) isCleaningUp;
#endif
Pointer(Fields) fields;
Field(Fields*) fields;

CompactCounters() { }
CompactCounters(T* host)
Expand Down
2 changes: 1 addition & 1 deletion lib/Runtime/Base/FunctionBody.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1731,7 +1731,7 @@ namespace Js
!this->GetSourceContextInfo()->IsDynamic() &&
this->m_scriptContext->DoUndeferGlobalFunctions())
{
this->GetUtf8SourceInfo()->UndeferGlobalFunctions([this](JsUtil::SimpleDictionaryEntry<Js::LocalFunctionId, Js::ParseableFunctionInfo*> func)
this->GetUtf8SourceInfo()->UndeferGlobalFunctions([this](const Utf8SourceInfo::DeferredFunctionsDictionary::EntryType& func)
{
Js::ParseableFunctionInfo *nextFunc = func.Value();
JavascriptExceptionObject* pExceptionObject = nullptr;
Expand Down
52 changes: 26 additions & 26 deletions lib/Runtime/Base/FunctionBody.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ namespace Js

typedef AuxPtrs<FunctionProxy, AuxPointerType> AuxPtrsT;
friend AuxPtrsT;
Pointer(AuxPtrsT) auxPtrs;
Field(AuxPtrsT*) auxPtrs;
void* GetAuxPtr(AuxPointerType e) const;
void* GetAuxPtrWithLock(AuxPointerType e) const;
void SetAuxPtr(AuxPointerType e, void* ptr);
Expand Down Expand Up @@ -1451,17 +1451,17 @@ namespace Js

protected:
// Static method(s)
static void SetDisplayName(const char16* srcName, Pointer(const char16)* destName, uint displayNameLength, ScriptContext * scriptContext, SetDisplayNameFlags flags = SetDisplayNameFlagsNone);
static void SetDisplayName(const char16* srcName, Field(const char16*)* destName, uint displayNameLength, ScriptContext * scriptContext, SetDisplayNameFlags flags = SetDisplayNameFlagsNone);
static bool SetDisplayName(const char16* srcName, const char16** destName, uint displayNameLength, ScriptContext * scriptContext, SetDisplayNameFlags flags = SetDisplayNameFlagsNone);
static bool IsConstantFunctionName(const char16* srcName);

protected:
PointerNoBarrier(ScriptContext) m_scriptContext; // Memory context for this function body
Pointer(Utf8SourceInfo) m_utf8SourceInfo;
FieldNoBarrier(ScriptContext*) m_scriptContext; // Memory context for this function body
Field(Utf8SourceInfo*) m_utf8SourceInfo;
// WriteBarrier-TODO: Consider changing this to NoWriteBarrierPtr, and skip tagging- also, tagging is likely unnecessary since that pointer in question is likely not resolvable
PointerNoBarrier(FunctionProxyPtr) m_referenceInParentFunction; // Reference to nested function reference to this function in the parent function body (tagged to not be actual reference)
Pointer(ScriptFunctionType) deferredPrototypeType;
Pointer(ProxyEntryPointInfo) m_defaultEntryPointInfo; // The default entry point info for the function proxy
FieldNoBarrier(FunctionProxyPtr*) m_referenceInParentFunction; // Reference to nested function reference to this function in the parent function body (tagged to not be actual reference)
Field(ScriptFunctionType*) deferredPrototypeType;
Field(ProxyEntryPointInfo*) m_defaultEntryPointInfo; // The default entry point info for the function proxy

Field(uint) m_functionNumber; // Per thread global function number

Expand Down Expand Up @@ -1771,7 +1771,7 @@ namespace Js
Field(bool) m_utf8SourceHasBeenSet; // start of UTF8-encoded source
Field(uint) m_sourceIndex; // index into the scriptContext's list of saved sources
#if DYNAMIC_INTERPRETER_THUNK
PointerNoBarrier(void) m_dynamicInterpreterThunk; // Unique 'thunk' for every interpreted function - used for ETW symbol decoding.
FieldNoBarrier(void*) m_dynamicInterpreterThunk; // Unique 'thunk' for every interpreted function - used for ETW symbol decoding.
#endif
Field(uint) m_cbStartOffset; // pUtf8Source is this many bytes from the start of the scriptContext's source buffer.

Expand All @@ -1783,10 +1783,10 @@ namespace Js

ULONG m_lineNumber;
ULONG m_columnNumber;
Pointer(const char16) m_displayName; // Optional name
Field(const char16*) m_displayName; // Optional name
uint m_displayNameLength;
Pointer(PropertyRecordList) m_boundPropertyRecords;
Pointer(NestedArray) nestedArray;
Field(PropertyRecordList*) m_boundPropertyRecords;
Field(NestedArray*) nestedArray;

public:
#if DBG
Expand Down Expand Up @@ -1988,16 +1988,16 @@ namespace Js
friend class ByteCodeBufferBuilder;

public:
PointerNoBarrier(SmallSpanSequence) pSpanSequence;
FieldNoBarrier(SmallSpanSequence*) pSpanSequence;

RegSlot frameDisplayRegister; // this register slot cannot be 0 so we use that sentinel value to indicate invalid
RegSlot objectRegister; // this register slot cannot be 0 so we use that sentinel value to indicate invalid
Pointer(ScopeObjectChain) pScopeObjectChain;
Pointer(ByteBlock) m_probeBackingBlock; // NULL if no Probes, otherwise a copy of the unmodified the byte-codeblock //Delay
Field(ScopeObjectChain*) pScopeObjectChain;
Field(ByteBlock*) m_probeBackingBlock; // NULL if no Probes, otherwise a copy of the unmodified the byte-codeblock //Delay
int32 m_probeCount; // The number of installed probes (such as breakpoints).

// List of bytecode offset for the Branch bytecode.
Pointer(AuxStatementData) m_auxStatementData;
Field(AuxStatementData*) m_auxStatementData;

SourceInfo():
frameDisplayRegister(0),
Expand All @@ -2012,18 +2012,18 @@ namespace Js
};

private:
Pointer(ByteBlock) byteCodeBlock; // Function byte-code for script functions
Pointer(FunctionEntryPointList) entryPoints;
Pointer(Var) m_constTable;
Pointer(void*) inlineCaches;
Field(ByteBlock*) byteCodeBlock; // Function byte-code for script functions
Field(FunctionEntryPointList*) entryPoints;
Field(Var*) m_constTable;
Field(void**) inlineCaches;
InlineCachePointerArray<PolymorphicInlineCache> polymorphicInlineCaches; // Contains the latest polymorphic inline caches
Pointer(PropertyId) cacheIdToPropertyIdMap;
Field(PropertyId*) cacheIdToPropertyIdMap;

#if DBG
#define InlineCacheTypeNone 0x00
#define InlineCacheTypeInlineCache 0x01
#define InlineCacheTypeIsInst 0x02
Pointer(byte) m_inlineCacheTypes;
Field(byte*) m_inlineCacheTypes;
#endif
public:
PropertyId * GetCacheIdToPropertyIdMap()
Expand Down Expand Up @@ -2149,7 +2149,7 @@ namespace Js
#ifdef IR_VIEWER
// whether IR Dump is enabled for this function (used by parseIR)
bool m_isIRDumpEnabled : 1;
Pointer(Js::DynamicObject) m_irDumpBaseObject;
Field(Js::DynamicObject*) m_irDumpBaseObject;
#endif /* IR_VIEWER */

Field(uint8) bailOnMisingProfileCount;
Expand Down Expand Up @@ -2181,14 +2181,14 @@ namespace Js
// copied in FunctionBody::Clone
//

PointerNoBarrier(Js::ByteCodeCache) byteCodeCache; // Not GC allocated so naked pointer
FieldNoBarrier(Js::ByteCodeCache*) byteCodeCache; // Not GC allocated so naked pointer
#ifdef ENABLE_DEBUG_CONFIG_OPTIONS
static bool shareInlineCaches;
#endif
Pointer(FunctionEntryPointInfo) defaultFunctionEntryPointInfo;
Field(FunctionEntryPointInfo*) defaultFunctionEntryPointInfo;

#if ENABLE_PROFILE_INFO
Pointer(DynamicProfileInfo) dynamicProfileInfo;
Field(DynamicProfileInfo*) dynamicProfileInfo;
#endif


Expand Down Expand Up @@ -2642,7 +2642,7 @@ namespace Js
bool IsSimpleJitOriginalEntryPoint() const;

#if DYNAMIC_INTERPRETER_THUNK
static BYTE GetOffsetOfDynamicInterpreterThunk() { return offsetof(FunctionBody, m_dynamicInterpreterThunk); }
static BYTE GetOffsetOfDynamicInterpreterThunk() { return static_cast<BYTE>(offsetof(FunctionBody, m_dynamicInterpreterThunk)); }
void* GetDynamicInterpreterEntryPoint() const
{
return m_dynamicInterpreterThunk;
Expand Down
4 changes: 2 additions & 2 deletions lib/Runtime/Language/EvalMapRecord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ namespace Js

bool TryGetValue(const Key& key, Value* value)
{
EntryRecord** entryRecord;
EntryRecord* const * entryRecord;
Key cachedKey;
int index;
bool success = dictionary->TryGetReference(key, &entryRecord, &index);
Expand Down Expand Up @@ -168,7 +168,7 @@ namespace Js

void Add(const Key& key, Value value)
{
EntryRecord** entryRecord;
EntryRecord* const * entryRecord;
int index;
bool success = dictionary->TryGetReference(key, &entryRecord, &index);
if (success && ((*entryRecord) != nullptr))
Expand Down
Loading