Skip to content

Commit

Permalink
Upgrade to WebkitGTK 2.24.2 and workaround __clear_cache issue o… (#114)
Browse files Browse the repository at this point in the history
# Summary

This PR includes three major changes:

1. Enable JIT back
    Community reported sensible performance drop from the no-JIT version, so I'd like to enable JIT back.

2. Upgrade to WebKitGTK 2.24.2.
    This seems to fix previous JSC crashes on Samsung S7 Edge.
    This version includes JIT new bytecode format as described from WebKit blog: 
    https://webkit.org/blog/9329/a-new-bytecode-format-for-javascriptcore/
    After the major change, x86 JIT is not supported and arm32 support was contributed by WebKit community (Thanks to Igalia).
   From my understanding, original JSC crashes happen at `operationLinkDirectCall()`. After the new bytecode format, there is no direct link call from Baseline JIT. Since we've disabled DFG JIT and FTL JIT, there's no call flow that will hit to `operationLinkDirectCall()`. That is why no more similar crash happens.

3. Workaround for ARM Cortex-A53 cache flush instruction issue:
    This is from V8's workaround and I believe it is worth to apply into JSC Android as well.
    https://codereview.chromium.org/1921173004
     ARM Cortex-A53 had some errata for original "cvau" instruction, and officially recommended to use 
    "civac" instruction instead.
     LLVM compiler-rt's `__clear_cache` still uses "cvau" and my patch replaced to "civac".

## Test Plan

1. Run measure scripts on my Samsung Note 5.
2. Provide an [experimented version](https://www.npmjs.com/package/@kudo-ci/jsc-android/v/245459.9000.0) for community who previously reported JSC crash and seems no more crashes happened.

## Measurement

Added "@kudo-ci/jsc-android@245459-no-dfg-jit" to previous measurement result.
The new result could compared to 241213-no-dfg-jit version.
There are some performance improvement from the comparison.
https://docs.google.com/spreadsheets/d/1hqX3ai-NCpN_J6YQDTKnKNBctWnMFA6EyOdVhPvwUas/edit#gid=193471288
<img width="735" alt="Screen Shot 2019-06-24 at 11 46 00 PM" src="https://user-images.githubusercontent.com/46429/60032978-44a1c480-96da-11e9-9eca-863aae3efe05.png">
<img width="413" alt="Screen Shot 2019-06-24 at 11 45 16 PM" src="https://user-images.githubusercontent.com/46429/60032980-44a1c480-96da-11e9-8f41-df30fa74235b.png">
<img width="414" alt="Screen Shot 2019-06-24 at 11 45 08 PM" src="https://user-images.githubusercontent.com/46429/60032981-453a5b00-96da-11e9-92a3-63318a7bb5fa.png">
<img width="427" alt="Screen Shot 2019-06-24 at 11 45 00 PM" src="https://user-images.githubusercontent.com/46429/60032982-453a5b00-96da-11e9-8d63-453ecc94c827.png">
<img width="426" alt="Screen Shot 2019-06-24 at 11 44 56 PM" src="https://user-images.githubusercontent.com/46429/60032983-453a5b00-96da-11e9-86c4-e5db0f6d86a9.png">
  • Loading branch information
Kudo authored and kmagiera committed Jun 25, 2019
1 parent c54f807 commit abdce96
Show file tree
Hide file tree
Showing 16 changed files with 126 additions and 106 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "jsc-android",
"version": "241213.2.0",
"version": "245459.0.0",
"description": "Pre-build version of JavaScriptCore to be used by React Native apps",
"repository": {
"type": "git",
Expand All @@ -26,7 +26,7 @@
"start": "./scripts/start.sh"
},
"config": {
"webkitGTK": "2.22.6",
"webkitGTK": "2.24.2",
"chromiumICUCommit": "b34251f8b762f8e2112a89c587855ca4297fed96"
}
}
2 changes: 1 addition & 1 deletion patches/jsc.patch
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
--- target-org/webkit/Source/JavaScriptCore/CMakeLists.txt 2019-04-27 00:04:34.000000000 +0800
+++ target/webkit/Source/JavaScriptCore/CMakeLists.txt 2019-04-27 00:06:00.000000000 +0800
@@ -1234,6 +1234,7 @@
@@ -1304,6 +1304,7 @@
install(TARGETS JavaScriptCore DESTINATION "${LIB_INSTALL_DIR}")
endif ()
endif ()
Expand Down
2 changes: 1 addition & 1 deletion patches/jsc_disable_icu.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
diff -aur target-org/webkit/Source/WTF/wtf/CMakeLists.txt target/webkit/Source/WTF/wtf/CMakeLists.txt
--- target-org/webkit/Source/WTF/wtf/CMakeLists.txt 2019-04-27 00:04:50.000000000 +0800
+++ target/webkit/Source/WTF/wtf/CMakeLists.txt 2019-04-27 00:14:29.000000000 +0800
@@ -475,7 +475,6 @@
@@ -482,7 +482,6 @@
list(APPEND WTF_SOURCES
unicode/CollatorDefault.cpp

Expand Down
4 changes: 2 additions & 2 deletions patches/jsc_features.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
diff -aur target-org/webkit/CMakeLists.txt target/webkit/CMakeLists.txt
--- target-org/webkit/CMakeLists.txt 2017-12-22 19:18:43.000000000 +0200
+++ target/webkit/CMakeLists.txt 2018-06-14 15:41:59.000000000 +0300
@@ -174,13 +174,13 @@
@@ -177,13 +177,13 @@
add_subdirectory(Tools)
endif ()

Expand Down Expand Up @@ -42,7 +42,7 @@ diff -aur target-org/webkit/Source/cmake/OptionsJSCOnly.cmake target/webkit/Sour
diff -aur target-org/webkit/Source/JavaScriptCore/CMakeLists.txt target/webkit/Source/JavaScriptCore/CMakeLists.txt
--- target-org/webkit/Source/JavaScriptCore/CMakeLists.txt 2017-05-02 21:13:03.000000000 +0200
+++ target/webkit/Source/JavaScriptCore/CMakeLists.txt 2017-07-11 11:34:55.962374878 +0200
@@ -1221,7 +1225,7 @@
@@ -1291,7 +1291,7 @@
)
target_include_directories(LLIntOffsetsExtractor SYSTEM PRIVATE ${JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES})

Expand Down
41 changes: 41 additions & 0 deletions patches/jsc_fix_arm64_jit_crash.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
diff -aur target-org/webkit/Source/JavaScriptCore/assembler/ARM64Assembler.h target/webkit/Source/JavaScriptCore/assembler/ARM64Assembler.h
--- target-org/webkit/Source/JavaScriptCore/assembler/ARM64Assembler.h 2019-06-18 21:49:21.000000000 +0800
+++ target/webkit/Source/JavaScriptCore/assembler/ARM64Assembler.h 2019-06-19 15:14:46.000000000 +0800
@@ -2863,7 +2863,36 @@

unsigned debugOffset() { return m_buffer.debugOffset(); }

-#if OS(LINUX) && COMPILER(GCC_COMPATIBLE)
+#if defined(CUSTOMIZE_REACT_NATIVE) && CPU(ARM64)
+ static inline void linuxPageFlush(uintptr_t start, uintptr_t end)
+ {
+ // NOTE(CUSTOMIZE_REACT_NATIVE): The code mostly copied from LLVM compiler-rt
+ // https://github.com/llvm-mirror/compiler-rt/blob/ff75f2a0260b1940436a483413091c5770427c04/lib/builtins/clear_cache.c#L142
+ // But only to modify "dc cvau" to "dc civac"
+
+ uint64_t xstart = (uint64_t)(uintptr_t)start;
+ uint64_t xend = (uint64_t)(uintptr_t)end;
+ uint64_t addr;
+
+ // Get Cache Type Info
+ uint64_t ctr_el0;
+ __asm __volatile("mrs %0, ctr_el0" : "=r"(ctr_el0));
+
+ // dc & ic instructions must use 64bit registers so we don't use
+ // uintptr_t in case this runs in an IPL32 environment.
+ const size_t dcache_line_size = 4 << ((ctr_el0 >> 16) & 15);
+ for (addr = xstart & ~(dcache_line_size - 1); addr < xend;
+ addr += dcache_line_size)
+ __asm __volatile("dc civac, %0" ::"r"(addr));
+ __asm __volatile("dsb ish");
+
+ const size_t icache_line_size = 4 << ((ctr_el0 >> 0) & 15);
+ for (addr = xstart & ~(icache_line_size - 1); addr < xend;
+ addr += icache_line_size)
+ __asm __volatile("ic ivau, %0" ::"r"(addr));
+ __asm __volatile("isb sy");
+ }
+#elif OS(LINUX) && COMPILER(GCC_COMPATIBLE)
static inline void linuxPageFlush(uintptr_t begin, uintptr_t end)
{
__builtin___clear_cache(reinterpret_cast<char*>(begin), reinterpret_cast<char*>(end));
71 changes: 0 additions & 71 deletions patches/jsc_fix_build_error_disable_dfg.patch

This file was deleted.

11 changes: 6 additions & 5 deletions patches/jsc_fix_build_error_getline.patch
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
diff -aur target-org/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp target/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp
--- target-org/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp 2018-11-11 23:05:48.000000000 +0800
+++ target/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp 2018-11-12 23:39:22.000000000 +0800
--- target-org/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp 2019-06-18 21:49:53.000000000 +0800
+++ target/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp 2019-06-18 22:44:39.000000000 +0800
@@ -23,6 +23,10 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
Expand All @@ -10,10 +10,11 @@ diff -aur target-org/webkit/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp target
+#if !defined(CUSTOMIZE_REACT_NATIVE)
+
#include "config.h"
#include "MemoryFootprint.h"
#include <wtf/MemoryFootprint.h>

@@ -107,3 +111,4 @@
@@ -100,3 +104,5 @@
}

}
} // namespace WTF
+
+#endif // !defined(CUSTOMIZE_REACT_NATIVE)
2 changes: 1 addition & 1 deletion patches/jsc_fix_build_error_log2.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
diff -aur target-org/webkit/Source/JavaScriptCore/Sources.txt target/webkit/Source/JavaScriptCore/Sources.txt
--- target-org/webkit/Source/JavaScriptCore/Sources.txt 2018-11-11 23:05:40.000000000 +0800
+++ target/webkit/Source/JavaScriptCore/Sources.txt 2018-11-12 00:03:26.000000000 +0800
@@ -1045,3 +1045,6 @@
@@ -1053,3 +1053,6 @@

// Derived Sources
yarr/YarrCanonicalizeUnicode.cpp
Expand Down
14 changes: 14 additions & 0 deletions patches/jsc_fix_build_error_miss_headers.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
diff -aur target-org/webkit/Source/JavaScriptCore/assembler/PerfLog.cpp target/webkit/Source/JavaScriptCore/assembler/PerfLog.cpp
--- target-org/webkit/Source/JavaScriptCore/assembler/PerfLog.cpp 2019-06-18 21:49:21.000000000 +0800
+++ target/webkit/Source/JavaScriptCore/assembler/PerfLog.cpp 2019-06-18 23:12:38.000000000 +0800
@@ -41,6 +41,10 @@
#include <wtf/PageBlock.h>
#include <wtf/ProcessID.h>

+#if defined(CUSTOMIZE_REACT_NATIVE)
+#include <array>
+#endif // defined(CUSTOMIZE_REACT_NATIVE)
+
namespace JSC {

namespace PerfLogInternal {
29 changes: 14 additions & 15 deletions patches/jsc_fix_build_error_mulodi4.patch
Original file line number Diff line number Diff line change
@@ -1,30 +1,29 @@
diff -aur target-org/webkit/Source/WTF/wtf/CheckedArithmetic.h target/webkit/Source/WTF/wtf/CheckedArithmetic.h
--- target-org/webkit/Source/WTF/wtf/CheckedArithmetic.h 2018-07-26 17:00:09.000000000 +0800
+++ target/webkit/Source/WTF/wtf/CheckedArithmetic.h 2019-04-12 12:03:55.000000000 +0800
@@ -317,7 +317,7 @@

--- target-org/webkit/Source/WTF/wtf/CheckedArithmetic.h 2019-06-18 21:49:53.000000000 +0800
+++ target/webkit/Source/WTF/wtf/CheckedArithmetic.h 2019-06-18 22:44:39.000000000 +0800
@@ -360,7 +360,7 @@

static inline bool multiply(LHS lhs, RHS rhs, ResultType& result) WARN_UNUSED_RETURN
{
-#if COMPILER(GCC_OR_CLANG)
+#if COMPILER(GCC_OR_CLANG) && CPU(ARM_THUMB2) && defined(NDEBUG)
-#if COMPILER(GCC_COMPATIBLE)
+#if COMPILER(GCC_COMPATIBLE) && CPU(ARM_THUMB2) && defined(NDEBUG)
ResultType temp;
if (__builtin_mul_overflow(lhs, rhs, &temp))
return false;
@@ -390,7 +390,7 @@

@@ -433,7 +433,7 @@
static inline bool multiply(LHS lhs, RHS rhs, ResultType& result) WARN_UNUSED_RETURN
{
-#if COMPILER(GCC_OR_CLANG)
+#if COMPILER(GCC_OR_CLANG) && CPU(ARM_THUMB2) && defined(NDEBUG)
-#if COMPILER(GCC_COMPATIBLE)
+#if COMPILER(GCC_COMPATIBLE) && CPU(ARM_THUMB2) && defined(NDEBUG)
ResultType temp;
if (__builtin_mul_overflow(lhs, rhs, &temp))
return false;
@@ -453,7 +453,7 @@

@@ -496,7 +496,7 @@
static inline bool multiply(int64_t lhs, int64_t rhs, ResultType& result)
{
-#if COMPILER(GCC_OR_CLANG)
+#if COMPILER(GCC_OR_CLANG) && CPU(ARM_THUMB2) && defined(NDEBUG)
-#if COMPILER(GCC_COMPATIBLE)
+#if COMPILER(GCC_COMPATIBLE) && CPU(ARM_THUMB2) && defined(NDEBUG)
ResultType temp;
if (__builtin_mul_overflow(lhs, rhs, &temp))
return false;
15 changes: 15 additions & 0 deletions patches/jsc_fix_build_error_statvfs.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
diff -aur target-org/webkit/Source/WTF/wtf/posix/FileSystemPOSIX.cpp target/webkit/Source/WTF/wtf/posix/FileSystemPOSIX.cpp
--- target-org/webkit/Source/WTF/wtf/posix/FileSystemPOSIX.cpp 2019-06-18 21:49:52.000000000 +0800
+++ target/webkit/Source/WTF/wtf/posix/FileSystemPOSIX.cpp 2019-06-18 22:58:46.000000000 +0800
@@ -45,6 +45,11 @@
#include <wtf/text/StringBuilder.h>
#include <wtf/text/WTFString.h>

+#if defined(CUSTOMIZE_REACT_NATIVE) && defined(__ANDROID__) && __ANDROID_API__ < 19
+#include <sys/vfs.h>
+#define statvfs statfs
+#endif // defined(CUSTOMIZE_REACT_NATIVE) && defined(__ANDROID__) && __ANDROID_API__ < 19
+
namespace WTF {

namespace FileSystemImpl {
6 changes: 3 additions & 3 deletions patches/jsc_fix_build_error_stringview.patch
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
diff -aur target-org/webkit/Source/WTF/wtf/unicode/CollatorDefault.cpp target/webkit/Source/WTF/wtf/unicode/CollatorDefault.cpp
--- target-org/webkit/Source/WTF/wtf/unicode/CollatorDefault.cpp 2014-03-15 05:08:27.000000000 +0100
+++ target/webkit/Source/WTF/wtf/unicode/CollatorDefault.cpp 2017-07-11 11:36:01.845264855 +0200
--- target-org/webkit/Source/WTF/wtf/unicode/CollatorDefault.cpp 2019-06-18 21:49:52.000000000 +0800
+++ target/webkit/Source/WTF/wtf/unicode/CollatorDefault.cpp 2019-06-18 22:44:39.000000000 +0800
@@ -28,12 +28,13 @@

#include "config.h"
#include "Collator.h"
#include <wtf/unicode/Collator.h>
+#include <StringView.h>

#if UCONFIG_NO_COLLATION
Expand Down
2 changes: 1 addition & 1 deletion patches/jsc_icu_integrate.patch
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ diff -aur target-org/webkit/Source/cmake/FindICU.cmake target/webkit/Source/cmak
diff -aur target-org/webkit/Source/JavaScriptCore/CMakeLists.txt target/webkit/Source/JavaScriptCore/CMakeLists.txt
--- target-org/webkit/Source/JavaScriptCore/CMakeLists.txt 2018-07-25 09:56:23.662494914 +0200
+++ target/webkit/Source/JavaScriptCore/CMakeLists.txt 2018-07-18 12:55:24.726736260 +0200
@@ -118,6 +118,8 @@
@@ -120,6 +120,8 @@
set(JavaScriptCore_LIBRARIES
WTF${DEBUG_SUFFIX}
${ICU_I18N_LIBRARIES}
Expand Down
2 changes: 1 addition & 1 deletion patches/jsc_startup_log_version.patch
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ diff -aur target-org/webkit/Source/JavaScriptCore/API/JSBase.cpp target/webkit/S
+}
--- target-org/webkit/Source/JavaScriptCore/CMakeLists.txt 2019-03-26 13:04:34.000000000 +0800
+++ target/webkit/Source/JavaScriptCore/CMakeLists.txt 2019-03-26 13:01:41.000000000 +0800
@@ -120,7 +120,9 @@
@@ -122,7 +122,9 @@
${ICU_I18N_LIBRARIES}
${ICU_LIBRARIES}
${ICU_DATA_LIBRARIES}
Expand Down
16 changes: 15 additions & 1 deletion scripts/compile/jsc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ else
BUILD_TYPE_FLAGS="-DDEBUG_FISSION=OFF"
fi

if [[ "$ARCH_NAME" = "i686" ]]
then
JSC_FEATURE_FLAGS=" \
-DENABLE_JIT=OFF \
-DENABLE_C_LOOP=ON \
"
else
JSC_FEATURE_FLAGS=" \
-DENABLE_JIT=ON \
-DENABLE_C_LOOP=OFF \
"
fi

$TARGETDIR/webkit/Tools/Scripts/build-webkit \
--jsc-only \
$BUILD_TYPE_CONFIG \
Expand Down Expand Up @@ -79,10 +92,11 @@ $TARGETDIR/webkit/Tools/Scripts/build-webkit \
-DCMAKE_VERBOSE_MAKEFILE=on \
-DENABLE_API_TESTS=OFF \
-DENABLE_SAMPLING_PROFILER=OFF \
-DENABLE_JIT=OFF \
-DENABLE_DFG_JIT=OFF \
-DENABLE_FTL_JIT=OFF \
-DUSE_SYSTEM_MALLOC=OFF \
-DJSC_VERSION=\"${JSC_VERSION}\" \
$JSC_FEATURE_FLAGS \
$BUILD_TYPE_FLAGS \
"

Expand Down
11 changes: 9 additions & 2 deletions scripts/patch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,15 @@ JSC_PATCHSET=(
# (However, NDK r19 fixed this)
"jsc_fix_build_error_mulodi4.patch"

# Fix build error if disabling DFG_JIT
"jsc_fix_build_error_disable_dfg.patch"
# statvfs is provided after NDK API level 19.
# Use statfs as fallback
"jsc_fix_build_error_statvfs.patch"

# Misc errors
"jsc_fix_build_error_miss_headers.patch"

# Workaround JIT crash on arm64, especially for Saumsung S7 Edge
"jsc_fix_arm64_jit_crash.patch"
)

if [[ "$I18N" = false ]]
Expand Down

0 comments on commit abdce96

Please sign in to comment.