Update Chakra JSI implementation #5710

vmoroz · 2020-08-12T23:26:35Z

While working on the ABI-safe JSI I have noticed a number of inefficiencies and bugs in the Chakra JSI implementation. Rather than adding them to the ABI-safe JSI implementation PR, I have put them instead into this PR.
The key issues/changes in this PR:

Added new ChakraEdgeRuntimeTests.cpp and testlib.cpp to MS.RN.ComponentTests to run JSI unit tests against the Chakra.
All the JSI unit tests are now passing without any exceptions as they are.
Removed the wide use of the ChakraObjectRef smart pointer that was eagerly calling JsAddRef/JsRelease even when we could avoid it. Using the plain JsValurRef, JsPropertyIdRef, or JsRef is safe as long as they are on the call stack or passed as parameters. We only need to use a smart pointer when we want to keep them in the heap allocated classes.
Introduced the ChakraApi class that wraps the Chakra APIs to:
- have common error handling mechanism;
- use standard library types such as std::wstring_view whenever it is possible;
- simplify the Chakra API usage.
ChakraRuntime inherits from the ChakraApi and overrides the exception throwing to throw the facebook::jsi::JSError and facebook::jsi::JSINativeException exceptions.
Fixed the HostFunction implementation to implement getHostFunction() instead of throwing exception.
Simplified implementation of HostObject support to avoid loading script and to cache the Proxy handler.
Cache all internally used JsPropertyIdRefs. It is to avoid any lookup when we need them.
Avoid using the JSI classes inside of the Chakra JSI implementation. This is to avoid any extra unneeded memory allocations and code indirections.
Avoid allocating memory for arguments when we call functions or HostObject/HostFunction. Use the call stack storage instead.

Outstanding issue: We currently leak memory when a JSI object is released on a wrong thread. JSI allows to release object in a different thread. Supporting it for Chakra will require using a DispatchQueue and a weak pointer to ChakraRuntime. We will fix it as a part of a different PR. The previous Chakra JSI crashes when it happens.

I have tried to measure the new Chakra JSI perf against the old one by running the Playground app. The results are inconclusive. We probably won 0.3-0.5%, but it is difficult to proof because timings maybe different 20% between the runs. The JSI seems to be a very small part of the overall work that the RN app does. But we know for sure that now we do less memory allocations for each JSI API call.

The key part of this change is that we have the full JSI interface implementation for Chakra and that it can pass all JSI unit tests.

Microsoft Reviewers: Open in CodeFlow

dannyvv · 2020-08-13T17:00:29Z

vnext/Microsoft.ReactNative.sln

@@ -138,20 +138,19 @@ Global
 		Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{da8b35b3-da00-4b02-bde6-6a397b3fd46b}*SharedItemsImports = 9
 		include\Include.vcxitems*{ef074ba1-2d54-4d49-a28e-5e040b47cd2e}*SharedItemsImports = 9
 		Chakra\Chakra.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4
-		JSI\Shared\JSI.Shared.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4


Shared [](start = 17, length = 6)

You might want to check with @julio 's change, he has a PR to remove JSI.Shared #Resolved

Yes, we have discussed it with him yesterday: I have moved the new unit tests to Microsoft.ReactNative.ComponentTests. He will move the other files if I merge before his PR. #Resolved

dannyvv · 2020-08-13T17:05:56Z

vnext/JSI/Shared/ChakraRuntime.h

+      if (JsRef ref = GetRef()) {
+        // JSI allows the destructor to be run on a random thread.
+        // To handle it correctly we must schedule a task to the thread associated
+        // with the Chakra context. Fir now we just leak the value.


Fir [](start = 36, length = 3)

Typo:

Suggested change

// with the Chakra context. Fir now we just leak the value.

For

``` #Resolved

dannyvv · 2020-08-13T17:07:50Z

vnext/JSI/Shared/ChakraRuntime.h

+    } catch (std::exception const &ex) {
+      ChakraVerifyElseThrow(false, (std::string{"Exception in "} + methodName + ": " + ex.what()).c_str());
+    } catch (...) {
+      ChakraVerifyElseThrow(false, (std::string{"Exception in "} + methodName + ": <unknown>").c_str());


ChakraVerifyElseThrow [](start = 6, length = 21)

Should there be a shortcut for this: ChakraThrow #Resolved

dannyvv · 2020-08-13T17:14:28Z

vnext/JSI/Shared/ChakraRuntime.cpp

-  std::vector<JsValueRef> argsWithThis = ConstructJsFunctionArguments(thisRef, argRefs);
-  assert(argsWithThis.size() <= (std::numeric_limits<unsigned short>::max)());
+  size_t jsArgCount = count + 1; // for 'this' argument.
+  ChakraVerifyElseThrow(jsArgCount <= MaxCallArgCount, "Argument count exceeds MaxCallArgCount");


MaxCallArgCount [](start = 38, length = 15)

Shouldn't this be a bit more dynamic? I.e. does JSI (or any egine for that) have a limit of 32 args?
Perhaps the code should be optimized for a few cases:
< 8 args, => on stack size 8
< 32 args => on stack size 32

= 32 args => on heap #Resolved

Great suggestion: I will see how to do it.

In reply to: 470106454 [](ancestors = 470106454)

I was able to specialize it for <= 8 and > 8 case

In reply to: 470186502 [](ancestors = 470186502,470106454)

dannyvv · 2020-08-13T17:15:42Z

vnext/JSI/Shared/ChakraRuntime.cpp

+  std::array<JsValueRef, MaxCallArgCount> jsArgs;
+  jsArgs[0] = m_undefinedValue;
+  for (size_t i = 0; i < count; ++i) {
+    jsArgs[i + 1] = ToJsValueRef(args[i]);


perhaps helper function to share with regular call... #Resolved

dannyvv · 2020-08-13T17:19:29Z

vnext/JSI/Shared/ChakraRuntime.cpp

+    throw facebook::jsi::JSError(*this, ToJsiValue(jsError));
+  } else {
+    std::ostringstream errorString;
+    errorString << "A call to Chakra API returned error code 0x" << std::hex << errorCode << '.';


std::hex << errorCode [](start = 68, length = 21)

Should this be in parenthesis? Or is << right associative? #Resolved

This is a correct syntax. It is left associate. To format number as hex we must first put std::hex into the stream. I think it changes the number format mode.

In reply to: 470111422 [](ancestors = 470111422)

dannyvv · 2020-08-13T17:22:26Z

vnext/JSI/Shared/ChakraRuntime.cpp

+  if (GetValueType(message) == JsValueType::JsString) {
+    // JSI unit tests expect V8 or JSC like message for stack overflow.
+    if (StringToPointer(message) == L"Out of stack space") {
+      SetProperty(jsError, m_propertyId.message, PointerToString(L"RangeError : Maximum call stack size exceeded"));


Just sharing: Is it me or is JSI inconsistent in the strings.. I see char in the method above (ThrowJSExceptoinOverride) and here we're using WCHAR for properties... #Resolved

Chakra is mostly UTF-16. We use it whenever possible to avoid the extra conversions. The standard exceptions and JSI API in general is mostly UTF-8. This is why we use UTF-8 to throw exceptions.

In reply to: 470114270 [](ancestors = 470114270)

dannyvv · 2020-08-13T17:23:37Z

vnext/JSI/Shared/ChakraRuntime.cpp

+  JsValueRef message = GetProperty(jsError, m_propertyId.message);
+  if (GetValueType(message) == JsValueType::JsString) {
+    // JSI unit tests expect V8 or JSC like message for stack overflow.
+    if (StringToPointer(message) == L"Out of stack space") {


ut of stack spa [](start = 39, length = 15)

Bigger question on RN: Will all developer errors be EN-US/1033 or will we have to localize these? #Resolved

This code is only to allow to pass the JSI tests. Localized messages are not used there. I do not know what is our approach to localized messages. I think RN or RNW do not support localization. Otherwise we would need to use a completely different set of APIs.

In reply to: 470115349 [](ancestors = 470115349)

aeulitz · 2020-08-13T18:49:14Z

vnext/JSI/Shared/ChakraApi.h

+#define ChakraVerifyElseCrash(condition, message)          \
+  do {                                                     \
+    if (!(condition)) {                                    \
+      assert(false && "Failed: " #condition && (message)); \


&& [](start = 19, length = 2)

For my understanding: What does the '&&' represent here? #Resolved

This is a common way to provide a message for the assert macro. It accepts only boolean and && allows to add a message because the string is always true. The assert then show the whole text given to it.

In reply to: 470172679 [](ancestors = 470172679)

aeulitz · 2020-08-13T19:05:56Z

vnext/JSI/Shared/JSI.Shared.vcxitems.filters

@@ -30,13 +27,12 @@
    <ClInclude Include="$(MSBuildThisFileDirectory)ChakraRuntimeFactory.h">
      <Filter>Header Files</Filter>
    </ClInclude>
+    <ClInclude Include="$(MSBuildThisFileDirectory)ChakraApi.h" />


Nit: Associate with "Header Files" filter? #Resolved

aeulitz · 2020-08-13T19:11:30Z

vnext/Microsoft.ReactNative.ComponentTests/Microsoft.ReactNative.ComponentTests.vcxproj

@@ -105,6 +105,8 @@
      </AdditionalIncludeDirectories>
      <UseFullPaths>true</UseFullPaths>
      <CallingConvention>Cdecl</CallingConvention>
+      <!-- RuntimeTypeInfo is required by JSI -->
+      <RuntimeTypeInfo>true</RuntimeTypeInfo>


Did your change introduce that, or was that always the case? #Resolved

It is required by JSI. The MS.RN already uses RTTI. This adds the same to the RN.dll. We need to change the JSI to avoid RTTI.

In reply to: 470186379 [](ancestors = 470186379)

I don't see MS.RN using RTTI via RuntimeTypeInfo, do you know where this is set?

In reply to: 470243854 [](ancestors = 470243854,470186379)

You are right: MS.RN explicitly disables RTTI. But, JSI needs it... I think it is an unpleasant JSI bug. Let me see how I can reduce the scope of RTTI here.

In reply to: 470472946 [](ancestors = 470472946,470243854,470186379)

aeulitz · 2020-08-13T19:17:17Z

vnext/Microsoft.ReactNative.ComponentTests/Microsoft.ReactNative.ComponentTests.vcxproj

+        C4245 - 'initializing': conversion from 'int' to 'unsigned __int64', signed/unsigned mismatch
+        C4389 - '==': signed/unsigned mismatch
+      -->
+      <DisableSpecificWarnings>4245;4389;$(DisableSpecificWarnings)</DisableSpecificWarnings>


4245;4389 [](start = 31, length = 9)

OOC, couldn't these warnings be quelled via static_casts in code? If so, what makes disabling them the better choice? #Resolved

The testlib.cpp is used from the RN npm. To fix it we need to change Hermes and RN code. This is why it is easier just to disable these warnings for now.

In reply to: 470189825 [](ancestors = 470189825)

can we just disable them at the point where they are produced instead of disabling them everywhere in the project?

In reply to: 470244479 [](ancestors = 470244479,470189825)

This file is coming from the NPM package. It is not part of our source tree. We cannot change the file. So, we are doing the next best thing: disable them for this file only in the project file. These warnings are not disabled for the whole project, they are disabled only for one file which comes from the RN NPM package. Note that they are inside of the ClCompile for testlib.cpp file only.

In reply to: 470473232 [](ancestors = 470473232,470244479,470189825)

gotcha, github was only showing me a snippet and i hadn't clicked the little arrows to show where this was in #Resolved

NickGerleman · 2020-08-13T19:21:37Z

vnext/Microsoft.ReactNative.ComponentTests/Microsoft.ReactNative.ComponentTests.vcxproj

@@ -158,6 +164,13 @@
    </ClCompile>
    <ClInclude Include="CommonReaderTest.h" />
    <ClInclude Include="pch/pch.h" />
+    <ClCompile Include="$(JSI_SourcePath)\jsi\test\testlib.cpp">


IIRC we have a separate source copy of JSI tests in our repo, from before they were published. Consider deleting those and changing them to include from JSI SourcePath like this is doing.

Related to that, I'm not familiar enough with existing project structure, but could we reconcile having multiple projects running JSI tests? #Resolved

I want to address it in future. The issue is that the old tests that we run for the Desktop environment are also used for V8 and to replace them with the stock tests there will require fixing the V8 JSI. It seems to be out of scope of this PR.

We will reconcile the tests, but we should do it in future after Julio removes the JSI projects. I think it will be done as we go towards the single RN core DLL.

In reply to: 470192372 [](ancestors = 470192372)

dannyvv

NikoAri · 2020-08-13T20:41:37Z

vnext/JSI/Shared/ChakraApi.h

+// Macros provide more flexibility to show assert and provide failure context.
+
+// Check condition and crash process if it fails.
+#define ChakraVerifyElseCrash(condition, message)          \


ChakraVerifyElseCrash [](start = 8, length = 21)

Why not call it "VerifyElseCrash"? #Resolved

I do not want to have a conflict here. As we are going toward reduced project structure - the effort driven by Julio - we can replace this one with the VerifyElseCrash. I cannot do it now because this code has no dependency on the Mso library.

In reply to: 470235462 [](ancestors = 470235462)

NikoAri · 2020-08-13T20:48:00Z

vnext/JSI/Shared/ChakraRuntime.h

+    void invalidate() noexcept override {}
+
+    JsRef GetRef() const noexcept {
+      return m_ref;


m_ref [](start = 13, length = 5)

(super nit) can we spell it out as m_jsRef? #Resolved

NikoAri · 2020-08-13T20:49:10Z

vnext/JSI/Shared/ChakraRuntime.h

+      bool isConstructCall,
+      JsValueRef *args,
+      unsigned short argCount,
+      void *callbackState) noexcept;


; [](start = 35, length = 1)

(nit) can we add empty line between these for readability? #Resolved

asklar · 2020-08-14T07:59:40Z

vnext/JSI/Shared/ChakraRuntime.h

+  // It avoids extra memory allocation by using an in-place storage.
+  // It uses ChakraPointerValueView that does nothing in the invalidate() method.
+  struct JsiValueView {
+    JsiValueView(JsValueRef jsValue);


final? #Resolved

asklar · 2020-08-14T07:59:47Z

vnext/JSI/Shared/ChakraRuntime.h

+  // If number of arguments is below or equal to MaxStackArgCount,
+  // then they are kept on call stack, otherwise arguments are allocated on heap.
+  struct JsValueArgs {
+    JsValueArgs(ChakraRuntime &rt, facebook::jsi::Value const &firstArg, Span<facebook::jsi::Value const> args);


final? #Resolved

asklar · 2020-08-14T08:00:16Z

vnext/JSI/Shared/ChakraRuntime.h

+
+   private:
+    size_t m_count{};
+    std::array<JsValueRef, MaxStackArgCount> m_stackArgs;


does this ever mutate? #Resolved

asklar · 2020-08-14T08:00:40Z

vnext/JSI/Shared/ChakraRuntime.h

+  // This class helps to use stack storage for passing arguments that must be temporary converted from
+  // JsValueRef to facebook::jsi::Value.
+  struct JsiValueViewArgs {
+    JsiValueViewArgs(JsValueRef *args, size_t argCount) noexcept;


final? #Resolved

asklar · 2020-08-14T08:00:56Z

vnext/JSI/Shared/ChakraRuntime.h

+  // PropNameIDView helps to use the stack storage for temporary conversion from
+  // JsPropertyIdRef to facebook::jsi::PropNameID.
+  struct PropNameIDView {
+    PropNameIDView(JsPropertyIdRef propertyId) noexcept;


final? #Resolved

# Conflicts: # vnext/JSI/Shared/ScriptStore.h

dannyvv · 2020-08-24T16:40:01Z

/azp run PR

azure-pipelines · 2020-08-24T16:40:20Z

Azure Pipelines successfully started running 1 pipeline(s).

# Conflicts: # vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp # vnext/JSI/Shared/ChakraRuntime.cpp # vnext/JSI/Shared/ChakraRuntime.h # vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp

# Conflicts: # vnext/JSI/Shared/ChakraRuntime.cpp

* Move UIManager to NM2 infra * formatting * Update x86 def file * Change files * Add win32 as a valid platform to platformcolorexample.win32 (#6255) * Fix switch events (#6264) * Switch was generating spurious events which could result on two events (on/off) being sent before the first event was fully handled * Change files * Implement code review recommendations * Make sure that m_updating is not left in wrong state if toggle switch null * Address further review comment * Update Chakra JSI implementation (#5710) * Update Chakra JSI implementation * Change files * Move new Chakra JSI Universal tests into MS.RN.ComponentTests * Address PR feedback * Address PR feedback * Fix x64 release compilation error * Add CreateRuntime and DisposeRuntime * Confirm to JSI API changes * Remove JSI_SourcePath * Allow having empty strings as JS object property name * applying package updates ***NO_CI*** * Bump @types/react-native from 0.63.25 to 0.63.26 (#6273) * Bump beachball from 1.37.0 to 1.38.0 (#6276) * Bump @types/jest from 26.0.14 to 26.0.15 (#6274) * Drop Issue Requirement from Override Manifest Schema Validation (#6263) * Drop Issue Requirement from Override Manifest Schema Validation Most types of overrrides require an issue number. This is done on the schema level, and is done incosistently per-type, where some allow no issues or LEGACY_FIXME, where others allow none. This was done to add barriers to forking, but ends up being a bit to broad/heavy-handed, and the differences between requirements of override types is confusing. This changes makes the schema uniform, dropping the special LEGACY_FIXME issue and universally allowing missing issues. To guide engineers to the right path, we still do require issues depending on types in the override prompt, but these can be altered manually. * Change files * Update copies to undefined issue during manifest generation * serializeBase cleanup * Bump babel-jest from 26.5.2 to 26.6.0 (#6272) * Add yargs-parser and node-fetch resolutions to pass component governance (#6270) Closes #6266 Closes #6267 * Fix Crash when Logbox shown on pre-19H1 (#6283) * Fix Crash when Logbox shown on pre-19H1 ActualHeight was added to UIElement in 19H1, but is being used in native code for Logbox/legacy redbox. This will also cause integration tests to crash on redbox, since they're running on debug build on Windows Server 2019. Replace usages with ActualHeight/ActualWidth. Initial thoughts are to not backport this, since it's dev only and has existed for a couple of versions. * Change files * Fix callbacks * formatting * Logging test fires an error, which shouldnt fail the integration tests * TextInput and ScrollViewer use some non-standard event names that I missed Co-authored-by: Patrick Boyd <[email protected]> Co-authored-by: gillpeacegood <[email protected]> Co-authored-by: Vladimir Morozov <[email protected]> Co-authored-by: React-Native-Windows Bot <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Nick Gerleman <[email protected]> Co-authored-by: Jon Thysell <[email protected]>

vmoroz added 2 commits August 12, 2020 15:52

Update Chakra JSI implementation

d188842

Change files

29cd2d8

vmoroz requested review from hansenyy, jonthysell, dannyvv, acoates-ms, tudorms, ZihanChen-MSFT and a team August 12, 2020 23:26

vmoroz requested a review from a team as a code owner August 12, 2020 23:26

vmoroz mentioned this pull request Aug 13, 2020

Initial ABI-safe JSI implementation #5461

Merged

vmoroz changed the title ~~Update Chkra JSI implementation~~ Update Chakra JSI implementation Aug 13, 2020

Move new Chakra JSI Universal tests into MS.RN.ComponentTests

377d147

dannyvv reviewed Aug 13, 2020

View reviewed changes

aeulitz reviewed Aug 13, 2020

View reviewed changes

NickGerleman reviewed Aug 13, 2020

View reviewed changes

dannyvv approved these changes Aug 13, 2020

View reviewed changes

NikoAri reviewed Aug 13, 2020

View reviewed changes

asklar reviewed Aug 14, 2020

View reviewed changes

vmoroz added 5 commits August 14, 2020 11:07

Address PR feedback

7a26340

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

5a4c47d

# Conflicts: # vnext/JSI/Shared/ScriptStore.h

Fix x64 release compilation error

d154e24

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

1268e4c

Add CreateRuntime and DisposeRuntime

da0f730

vmoroz mentioned this pull request Aug 17, 2020

Update Chakra JSI implementation (2) #5751

Closed

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

f279dd5

ghost added the Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) label Sep 15, 2020

dannyvv removed the Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) label Sep 17, 2020

chrisglein added Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) and removed Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) labels Sep 17, 2020

vmoroz added 5 commits September 29, 2020 16:37

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

5482d04

# Conflicts: # vnext/JSI/Desktop/ChakraJsiRuntime_core.cpp # vnext/JSI/Shared/ChakraRuntime.cpp # vnext/JSI/Shared/ChakraRuntime.h # vnext/JSI/Universal/ChakraJsiRuntime_edgemode.cpp

Confirm to JSI API changes

db6837d

Remove JSI_SourcePath

35f1d17

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

67d7553

# Conflicts: # vnext/JSI/Shared/ChakraRuntime.cpp

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

aa85781

acoates-ms mentioned this pull request Oct 19, 2020

Move UIManager to NM2 Infra #6268

Merged

vmoroz added 2 commits October 19, 2020 14:51

Merge remote-tracking branch 'upstream/master' into PR/FixChakraJSI

a69724b

Allow having empty strings as JS object property name

664da99

ghost merged commit a40f3fb into microsoft:master Oct 20, 2020

nichamp mentioned this pull request Feb 22, 2021

RNW 0.64 compatibility issues microsoft/react-native-winrt#30

Closed

asklar mentioned this pull request Mar 4, 2021

Remove HostObject Jsi exceptions #7274

Merged

This pull request was closed.

	// with the Chakra context. Fir now we just leak the value.
	For
	``` #Resolved

Update Chakra JSI implementation #5710

Update Chakra JSI implementation #5710

Conversation

vmoroz commented Aug 12, 2020 • edited Loading

Microsoft Reviewers: Open in CodeFlow

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

vmoroz Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannyvv Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aeulitz Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aeulitz Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

aeulitz Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aeulitz Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asklar Aug 14, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

NickGerleman Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannyvv left a comment

Choose a reason for hiding this comment

NikoAri Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NikoAri Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

NikoAri Aug 13, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

asklar Aug 14, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

asklar Aug 14, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

asklar Aug 14, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

asklar Aug 14, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

asklar Aug 14, 2020 • edited by vmoroz Loading

Choose a reason for hiding this comment

dannyvv commented Aug 24, 2020

azure-pipelines bot commented Aug 24, 2020

vmoroz commented Aug 12, 2020 •

edited

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

vmoroz Aug 13, 2020 •

edited

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

dannyvv Aug 13, 2020 •

edited by vmoroz

Loading

aeulitz Aug 13, 2020 •

edited by vmoroz

Loading

aeulitz Aug 13, 2020 •

edited by vmoroz

Loading

aeulitz Aug 13, 2020 •

edited by vmoroz

Loading

aeulitz Aug 13, 2020 •

edited by vmoroz

Loading

asklar Aug 14, 2020 •

edited by vmoroz

Loading

NickGerleman Aug 13, 2020 •

edited by vmoroz

Loading

NikoAri Aug 13, 2020 •

edited by vmoroz

Loading

NikoAri Aug 13, 2020 •

edited by vmoroz

Loading

NikoAri Aug 13, 2020 •

edited by vmoroz

Loading

asklar Aug 14, 2020 •

edited by vmoroz

Loading

asklar Aug 14, 2020 •

edited by vmoroz

Loading

asklar Aug 14, 2020 •

edited by vmoroz

Loading

asklar Aug 14, 2020 •

edited by vmoroz

Loading

asklar Aug 14, 2020 •

edited by vmoroz

Loading