From d2acfdec5c85fe015ca5faa047df3777de987778 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 4 Jul 2023 14:36:25 +0700 Subject: [PATCH] fix(developer): improve kmcmplib/kmw compiler performance Fixes #9154. Improves performance in both kmcmplib and in kmc-kmn/kmw-compiler. After these changes are applied, vietnamese_telex keyboard builds in around 6 seconds on my machine (down from over 2 minutes). This is certainly only the surface of performance improvements we could make. Addresses excessive memory reallocations in kmcmplib, by reallocating store and key arrays in large chunks of 100 items rather than reallocating on each new item added. This resulted in a 250x speed-up on functions such as `AddStore` in WASM build. Some careful modulus arithmetic was needed for resizing the key array, which can be grown in larger increments. Resolved excessive zeroing out of destination buffer in u16ncpy, which was being called with an 8kb destination buffer on every line of the file. In the kmw side, vast majority of the performance cost was in copying of buffers while loading a .kmx into memory, rather than creating views into the memory. Essentially replaced this pattern: ``` return this.rString.fromBuffer(source.slice(offset)); ``` with: ``` const data = new Uint8Array(source.buffer, source.byteOffset + offset); return this.rString.fromBuffer(data); ``` --- common/web/types/src/kmx/kmx-file-reader.ts | 16 +++- developer/src/kmcmplib/src/CasedKeys.cpp | 17 ++-- developer/src/kmcmplib/src/Compiler.cpp | 96 ++++++++++--------- .../src/kmcmplib/src/UnreachableRules.cpp | 2 +- developer/src/kmcmplib/src/kmx_u16.cpp | 5 +- developer/src/kmcmplib/src/meson.build | 20 +++- 6 files changed, 88 insertions(+), 68 deletions(-) diff --git a/common/web/types/src/kmx/kmx-file-reader.ts b/common/web/types/src/kmx/kmx-file-reader.ts index 57f58751d1..083717ffe7 100644 --- a/common/web/types/src/kmx/kmx-file-reader.ts +++ b/common/web/types/src/kmx/kmx-file-reader.ts @@ -15,7 +15,12 @@ export class KmxFileReader { // string ('') return null; } - return this.rString.fromBuffer(source.slice(offset)); + // The following two lines are equivalent to : + // return this.rString.fromBuffer(source.slice(offset)); + // but is much faster because it is a read-only view into + // the data rather than a copy + const data = new Uint8Array(source.buffer, source.byteOffset + offset); + return this.rString.fromBuffer(data); } private isValidCodeUse(s: string, keyboard: KEYBOARD): boolean { @@ -99,7 +104,8 @@ export class KmxFileReader { private readGroupsAndRules(binaryKeyboard: BUILDER_COMP_KEYBOARD, kmx: KMXFile, source: Uint8Array, result: KEYBOARD) { let offset = binaryKeyboard.dpGroupArray; for (let i = 0; i < binaryKeyboard.cxGroupArray; i++) { - let binaryGroup = kmx.COMP_GROUP.fromBuffer(source.slice(offset)); + const data = new Uint8Array(source.buffer, source.byteOffset + offset); + let binaryGroup = kmx.COMP_GROUP.fromBuffer(data); let group = new GROUP(); group.dpMatch = this.readString(source, binaryGroup.dpMatch); group.dpName = this.readString(source, binaryGroup.dpName); @@ -109,7 +115,8 @@ export class KmxFileReader { let keyOffset = binaryGroup.dpKeyArray; for (let j = 0; j < binaryGroup.cxKeyArray; j++) { - let binaryKey = kmx.COMP_KEY.fromBuffer(source.slice(keyOffset)); + const keyData = new Uint8Array(source.buffer, source.byteOffset + keyOffset); + let binaryKey = kmx.COMP_KEY.fromBuffer(keyData); let key = new KEY(); key.Key = binaryKey.Key; key.Line = binaryKey.Line; @@ -128,7 +135,8 @@ export class KmxFileReader { private readStores(binaryKeyboard: BUILDER_COMP_KEYBOARD, kmx: KMXFile, source: Uint8Array, result: KEYBOARD): void { let offset = binaryKeyboard.dpStoreArray; for (let i = 0; i < binaryKeyboard.cxStoreArray; i++) { - let binaryStore = kmx.COMP_STORE.fromBuffer(source.slice(offset)); + const data = new Uint8Array(source.buffer, source.byteOffset + offset); + let binaryStore = kmx.COMP_STORE.fromBuffer(data); let store = new STORE(); store.dwSystemID = binaryStore.dwSystemID; store.dpName = this.readString(source, binaryStore.dpName); diff --git a/developer/src/kmcmplib/src/CasedKeys.cpp b/developer/src/kmcmplib/src/CasedKeys.cpp index 4e1e04723e..759a497d14 100644 --- a/developer/src/kmcmplib/src/CasedKeys.cpp +++ b/developer/src/kmcmplib/src/CasedKeys.cpp @@ -14,6 +14,8 @@ namespace kmcmp { extern KMX_BOOL FMnemonicLayout; // TODO: these globals should be consolidated one day } +bool resizeKeyArray(PFILE_GROUP gp, int increment = 1); + KMX_DWORD ExpandCapsRule(PFILE_GROUP gp, PFILE_KEY kpp, PFILE_STORE sp); KMX_DWORD VerifyCasedKeys(PFILE_STORE sp) { @@ -127,17 +129,14 @@ KMX_DWORD ExpandCapsRule(PFILE_GROUP gp, PFILE_KEY kpp, PFILE_STORE sp) { } // This key is modified by Caps Lock, so we need to duplicate this rule - PFILE_KEY k = new FILE_KEY[gp->cxKeyArray + 1]; - if (!k) return CERR_CannotAllocateMemory; - memcpy(k, gp->dpKeyArray, gp->cxKeyArray * sizeof(FILE_KEY)); - - kpp = &k[(int)(kpp - gp->dpKeyArray)]; - - delete[] gp->dpKeyArray; - gp->dpKeyArray = k; + int offset = (int)(kpp - gp->dpKeyArray); + if(!resizeKeyArray(gp)) { + return CERR_CannotAllocateMemory; + } + kpp = &gp->dpKeyArray[offset]; gp->cxKeyArray++; - k = &k[gp->cxKeyArray - 1]; + PFILE_KEY k = &gp->dpKeyArray[gp->cxKeyArray - 1]; k->dpContext = new KMX_WCHAR[u16len(kpp->dpContext) + 1]; k->dpOutput = new KMX_WCHAR[u16len(kpp->dpOutput) + 1]; u16cpy(k->dpContext, kpp->dpContext ); // copy the context. diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index dad7ec9010..02990bc324 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -167,6 +167,9 @@ KMX_DWORD process_expansion(PFILE_KEYBOARD fk, PKMX_WCHAR q, PKMX_WCHAR tstr, in KMX_BOOL IsValidKeyboardVersion(KMX_WCHAR *dpString); +bool resizeStoreArray(PFILE_KEYBOARD fk); +bool resizeKeyArray(PFILE_GROUP gp, int increment = 1); + const KMX_WCHAR * LineTokens[] = { u"SVNBHBGMNSCCLLCMLB", u"store", u"VERSION ", u"NAME ", u"BITMAP ", u"HOTKEY ", u"begin", u"group", u"match", u"nomatch", @@ -732,16 +735,9 @@ KMX_DWORD ProcessStoreLine(PFILE_KEYBOARD fk, PKMX_WCHAR p) if (!StoreTokens[i]) return CERR_InvalidSystemStore; } - sp = new FILE_STORE[fk->cxStoreArray + 1]; - if (!sp) return CERR_CannotAllocateMemory; - - if (fk->dpStoreArray) - { - memcpy(sp, fk->dpStoreArray, sizeof(FILE_STORE) * fk->cxStoreArray); - delete[] fk->dpStoreArray; + if(!resizeStoreArray(fk)) { + return CERR_CannotAllocateMemory; } - - fk->dpStoreArray = sp; sp = &fk->dpStoreArray[fk->cxStoreArray]; sp->line = kmcmp::currentLine; @@ -790,19 +786,47 @@ KMX_DWORD ProcessStoreLine(PFILE_KEYBOARD fk, PKMX_WCHAR p) return CheckForDuplicateStore(fk, sp); } +bool resizeStoreArray(PFILE_KEYBOARD fk) { + if(fk->cxStoreArray % 100 == 0) { + PFILE_STORE sp = new FILE_STORE[fk->cxStoreArray + 100]; + if (!sp) return false; + + if (fk->dpStoreArray) + { + memcpy(sp, fk->dpStoreArray, sizeof(FILE_STORE) * fk->cxStoreArray); + delete[] fk->dpStoreArray; + } + + fk->dpStoreArray = sp; + } + return true; +} + +/** + * reallocates the key array in increments of 100 + */ +bool resizeKeyArray(PFILE_GROUP gp, int increment) { + if((gp->cxKeyArray + increment - 1) % 100 < increment) { + PFILE_KEY kp = new FILE_KEY[((gp->cxKeyArray + increment)/100 + 1) * 100]; + if (!kp) return false; + if (gp->dpKeyArray) + { + memcpy(kp, gp->dpKeyArray, gp->cxKeyArray * sizeof(FILE_KEY)); + delete[] gp->dpKeyArray; + } + + gp->dpKeyArray = kp; + } + return true; +} + KMX_DWORD AddStore(PFILE_KEYBOARD fk, KMX_DWORD SystemID, const KMX_WCHAR * str, KMX_DWORD *dwStoreID) { PFILE_STORE sp; - sp = new FILE_STORE[fk->cxStoreArray + 1]; - if (!sp) return CERR_CannotAllocateMemory; - - if (fk->dpStoreArray) - { - memcpy(sp, fk->dpStoreArray, sizeof(FILE_STORE) * fk->cxStoreArray); - delete[] fk->dpStoreArray; + if(!resizeStoreArray(fk)) { + return CERR_CannotAllocateMemory; } - fk->dpStoreArray = sp; sp = &fk->dpStoreArray[fk->cxStoreArray]; sp->line = kmcmp::currentLine; @@ -832,16 +856,9 @@ KMX_DWORD AddDebugStore(PFILE_KEYBOARD fk, KMX_WCHAR const * str) KMX_WCHAR tstr[16]; u16sprintf(tstr, _countof(tstr), L"%d", kmcmp::currentLine); // I3481 - sp = new FILE_STORE[fk->cxStoreArray + 1]; - if (!sp) return CERR_CannotAllocateMemory; - - if (fk->dpStoreArray) - { - memcpy(sp, fk->dpStoreArray, sizeof(FILE_STORE) * fk->cxStoreArray); - delete[] fk->dpStoreArray; + if(!resizeStoreArray(fk)) { + return CERR_CannotAllocateMemory; } - - fk->dpStoreArray = sp; sp = &fk->dpStoreArray[fk->cxStoreArray]; safe_wcsncpy(sp->szName, (PKMX_WCHAR) str, SZMAX_STORENAME); @@ -1463,15 +1480,10 @@ KMX_DWORD ProcessKeyLineImpl(PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX_BOOL IsUnico } } - kp = new FILE_KEY[gp->cxKeyArray + 1]; - if (!kp) return CERR_CannotAllocateMemory; - if (gp->dpKeyArray) - { - memcpy(kp, gp->dpKeyArray, gp->cxKeyArray * sizeof(FILE_KEY)); - delete[] gp->dpKeyArray; + if(!resizeKeyArray(gp)) { + return CERR_CannotAllocateMemory; } - gp->dpKeyArray = kp; kp = &gp->dpKeyArray[gp->cxKeyArray]; gp->cxKeyArray++; @@ -1581,14 +1593,13 @@ KMX_DWORD ExpandKp(PFILE_KEYBOARD fk, PFILE_KEY kpp, KMX_DWORD storeIndex) and set the keystroke to the appropriate character in the store. */ - k = new FILE_KEY[gp->cxKeyArray + nchrs - 1]; - if (!k) return CERR_CannotAllocateMemory; - memcpy(k, gp->dpKeyArray, gp->cxKeyArray * sizeof(FILE_KEY)); + int offset = (int)(kpp - gp->dpKeyArray); - kpp = &k[(int)(kpp - gp->dpKeyArray)]; + if (!resizeKeyArray(gp, nchrs)) { + return CERR_CannotAllocateMemory; + } - delete[] gp->dpKeyArray; - gp->dpKeyArray = k; + kpp = &gp->dpKeyArray[offset]; gp->cxKeyArray += nchrs - 1; for (k = kpp, n = 0, pn = sp->dpString; *pn; pn = incxstr(pn), k++, n++) @@ -3132,14 +3143,7 @@ KMX_DWORD ReadLine(KMX_BYTE* infile, int sz, int& offset, PKMX_WCHAR wstr, KMX_B return CERR_EndOfFile; } - // neccessary to add this block for using on non-windows platforms (removes all \r for platforms that use \n instead of \r\n) - for (p = str, n = 0; n < len; n++, p++) { - if (*p == L'\r') - *p = L' '; - } - // \r is still left in this block even though Linux doesn`t use \r. - // This is to ensure to still have a working windows-only-version for (p = str, n = 0; n < len; n++, p++) { if (currentQuotes != 0) diff --git a/developer/src/kmcmplib/src/UnreachableRules.cpp b/developer/src/kmcmplib/src/UnreachableRules.cpp index d8980599a2..9d114a80a4 100644 --- a/developer/src/kmcmplib/src/UnreachableRules.cpp +++ b/developer/src/kmcmplib/src/UnreachableRules.cpp @@ -14,7 +14,7 @@ #include "UnreachableRules.h" namespace kmcmp { - std::wstring MakeHashKeyFromFileKey(PFILE_KEY kp) { + std::wstring MakeHashKeyFromFileKey(PFILE_KEY kp) { std::wstringstream key; key << kp->Key << "," << kp->ShiftFlags << ","; if (kp->dpContext) { diff --git a/developer/src/kmcmplib/src/kmx_u16.cpp b/developer/src/kmcmplib/src/kmx_u16.cpp index 01ab4063ed..c73796e9b6 100644 --- a/developer/src/kmcmplib/src/kmx_u16.cpp +++ b/developer/src/kmcmplib/src/kmx_u16.cpp @@ -166,9 +166,8 @@ const KMX_WCHAR * u16ncpy(KMX_WCHAR *dst, const KMX_WCHAR *src, size_t max) { *dst++ = *src++; max--; } - while(max > 0) { - *dst++ = 0; - max--; + if(max > 0) { + *dst = 0; } return o; } diff --git a/developer/src/kmcmplib/src/meson.build b/developer/src/kmcmplib/src/meson.build index 631631e2bd..02e7cca626 100644 --- a/developer/src/kmcmplib/src/meson.build +++ b/developer/src/kmcmplib/src/meson.build @@ -26,12 +26,22 @@ name_suffix = [] if cpp_compiler.get_id() == 'emscripten' # wasm-exceptions supported in Node 18+, Chrome 95+, Firefox 100+, Safari 15.2+ - flags += ['-fwasm-exceptions'] - # For additional (slow) sanitize, add: '-fsanitize=undefined', '-fsanitize=address' - lib_links = ['--whole-archive', '-sALLOW_MEMORY_GROWTH', '-sMODULARIZE', '-sEXPORT_ES6'] - # Alternative to sanitize is adding '-sASSERTIONS', '-sSAFE_HEAP' to lib_links - links += ['-fwasm-exceptions', '--bind', '-sEXPORTED_RUNTIME_METHODS=[\'UTF8ToString\']'] + + # For additional (slow) sanitize, add: # Additional sanitize required on both flags and links: '-fsanitize=undefined', '-fsanitize=address' + #sanitize = ['-fsanitize=undefined', '-fsanitize=address'] + sanitize = [] + + flags += ['-fwasm-exceptions'] + sanitize + + lib_links = ['--whole-archive', '-sALLOW_MEMORY_GROWTH', '-sMODULARIZE', '-sEXPORT_ES6'] + links += ['-fwasm-exceptions', '--bind', '-sEXPORTED_RUNTIME_METHODS=[\'UTF8ToString\']'] + sanitize + + # Alternative to sanitize is adding '-sASSERTIONS', '-sSAFE_HEAP' to lib_links: + # lib_links += ['-sASSERTIONS', '-sSAFE_HEAP'] + + # For profiling, add: + # lib_links += ['--profiling-funcs', '-sDEMANGLE_SUPPORT=1'] endif icu = subproject('icu-for-uset', default_options: [ 'default_library=static', 'cpp_std=c++17', 'warning_level=0', 'werror=false'])