Improve performance - #1631
Open
ksh8281 wants to merge 7 commits into
Open
Conversation
…to 3 slots The MRU front-entry check (added in 6c95698) only looks at index 0, on the assumption that a callsite's cache tends to be re-hit at whatever shape was inserted most recently. Fresh telemetry on Preact's VNode-construction workload showed that assumption failing hard for this tier: of ~2.06M front-entry misses, essentially all of them (~97.6%) resolved at scan position 1 or 2, not deeper -- shape churn here commonly rotates through a handful of distinct shapes per callsite rather than genuinely reusing the last one, so a single-slot check almost never hits and nearly every access was paying the NEVER_INLINE slow-path call anyway. Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
…in Set IC Set's inline cache only ever cached plain-data-and-writable own-property writes; any found property that turned out to be an accessor, a native getter/setter data property, or non-writable caused the callsite to permanently disable its IC (GiveUp: clear the cache, never try again) and fall back to the fully generic, fully-uncached property-set path forever after. This was asymmetric with Get's Complex tier, which already caches non-plain-data properties via getOwnNonPlainDataPropertyUtilForObject. Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
setObjectPreComputedCaseOperationCacheMiss's "property not found, cache the transition chain" branch walks the receiver's prototype chain checking, for each prototype, whether it's safe to cache (inline-cacheable) and whether the chain has already hit its depth cap. Commit 86525d1 added the depth check but folded it into the existing `!UNLIKELY(obj->isInlineCacheable())` condition as `!UNLIKELY(A || B)` instead of `UNLIKELY(!A || B)` -- by De Morgan that's `!A && !B`, the opposite of the intended "bail if not cacheable OR too deep": it only bails when the prototype is NOT inline-cacheable AND the chain is still under the cap, and otherwise keeps walking (and caching) past inlineCacheProtoTraverseMaxCount with no bound other than the real prototype chain's own length. Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
Both job-drain loops (evalScript's post-script drain and the 262-agent
worker loop) called waitEventFromAnotherThread(10) unconditionally on
every pass, even when a same-thread job was already pending. In a
single-threaded script that event never arrives, so every microtask
drained through these loops paid up to a 10ms wall-clock stall for
nothing.
A 200-link .then() chain took ~6s wall clock (0.04s CPU) before this
fix. Only block waiting for another thread's event when there is no
same-thread job ready to run right now.
Follow-up fix folded in here: skipping the cross-thread wait whenever a
same-thread job is pending starves an already-completed cross-thread
event (e.g. an expired Atomics.wait/waitAsync timeout) indefinitely if
the same-thread job queue keeps refilling itself -- a `setTimeout(fn, 0)`
polling loop, which is exactly the pattern test262's
Atomics/waitAsync/true-for-timeout.js and friends use. hasPendingJob()
never goes false in that case, so the cross-thread check never ran at
all. This surfaced as 6 test262 waitAsync failures that had been wrongly
written off as a pre-existing/known baseline flake in earlier
verification of this same change, instead of recognized as a regression
it introduced.
Fixed by adding VMInstance::hasCompletedJobFromAnotherThread() (plus its
EscargotPublic.h/.cpp wrapper), a non-blocking peek at whether a
cross-thread job has already completed. Both job-drain loops now use it
when a same-thread job is pending, instead of skipping the cross-thread
check entirely: zero added latency in the common case (no blocking call
happens), and an already-completed cross-thread event is picked up on
the very next loop iteration instead of being starved forever.
Verified: test262 Atomics/waitAsync/{true-for-timeout,
returns-result-object-value-is-promise-resolves-to-timed-out,
bigint/true-for-timeout} all pass now; promise-benchmark.js's 200-link
.then() chain still completes in ~0.06s wall clock (no stall
reintroduced).
Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
Reading arguments[i] or arguments.length currently always forces materialization of an ArgumentsObject, even when the read could be answered directly from the call frame. Add a LoadArgumentsElement opcode (merged .length + indexed-element query, since both need the same "is there already a materialized ArgumentsObject" check) that: - for .length, returns argc directly when no ArgumentsObject exists yet, else defers to the real object (so length-redefinition etc. keeps working). - for arguments[i], returns argv[i] directly without materializing when index is >= parameterCount and < argc -- this range can never alias a mapped-arguments parameter binding, so bypassing the object is observably identical to going through it, for both mapped and unmapped arguments objects. Anything outside that range (or once an ArgumentsObject already exists) falls back to the normal path. Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
…ands Both ADD_CUSTOM_COMMAND compile invocations for the N-API test addons (js-native-api/node-api upstream TCs and the custom symbol-verify addon) passed only -DNAPI_VERSION=10, but the additive Node.js-specific declarations they use (node_api_create_object_with_properties, node_api_set_prototype, node_api_post_finalizer, node_api_is_sharedarraybuffer) are guarded by `#ifdef NAPI_EXPERIMENTAL` in the vendored test/napi-tc/src/js_native_api.h, not by NAPI_VERSION. Without the flag those addons fail to compile with implicit-declaration errors. Added -DNAPI_EXPERIMENTAL -DNODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT to both compile commands, matching what a known-working N-API checkout already uses. Pre-existing gap from the N-API merge, unrelated to any other change on this branch -- found while trying to get a clean test.sh run. Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
…ecrement
Adding fast-path opcodes grows interpret()'s dispatch table and hurts
icache; the counter-move is collapsing genuinely redundant opcodes
(near-duplicate handlers differing by one field) back down.
- JumpIfTrue + JumpIfFalse -> JumpIfBoolean(shouldNegate, registerIndex).
Identical handlers but for one negation -- exactly the shape
JumpIfUndefinedOrNull already collapsed into one opcode via its own
m_shouldNegate flag. Same convention: result ^ shouldNegate. No
bytecode size change (same two fields either way), ~15 callsites
across src/parser/ast/*.h updated to construct/peek/lastCodePosition
the merged type with the right shouldNegate value.
- Increment/Decrement absorb ToNumericIncrement/ToNumericDecrement.
Increment(src,dst) (prefix ++i) and ToNumericIncrement(src,storeIndex,dst)
(postfix i++) were the same op with one extra field. Increment/Decrement
gain an m_storeIndex field (REGISTER_LIMIT sentinel for "no postfix
store", the same convention already used for m_receiverIndex and
LoadArgumentsElement's m_indexRegisterIndex) and absorb the two
ToNumeric* opcodes outright -- 2 opcodes removed, not renamed. Handler
merge is exactly the prior split logic gated on the sentinel check.
Only 12 callsites, all in UpdateExpression{Increment,Decrement}
{Prefix,Postfix}Node.h, none needing lastCodePosition/peekCode.
Signed-off-by: Seonghyun Kim <sh8281.kim@samsung.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.