From e1a44fca241817ce68e88dc1914e1d13f7a565b7 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 30 Sep 2026 16:30:24 -0700 Subject: [PATCH 1/4] process: let inline caches work on process.env, process.argv and process.execArgv Each first read of an environment variable changed the structure of process.env. After 128 changes JSC made it a dictionary, and reads were no longer cached. The values are now plain data properties from the start, on one structure. process.argv and process.execArgv called a native getter on every read. They are now lazy data properties, as in Node.js. A write to process.env from the same site could skip the conversion to a string, because the inline cache stored the value directly. Writes are no longer cached. --- bench/snippets/process-env.mjs | 28 ++++ src/jsc/bindings/BunProcess.cpp | 103 +++--------- src/jsc/bindings/BunProcess.h | 5 - src/jsc/bindings/InspectorLifecycleAgent.cpp | 5 +- src/jsc/bindings/JSEnvironmentVariableMap.cpp | 129 ++++++--------- src/jsc/bindings/JSEnvironmentVariableMap.h | 3 + src/jsc/bindings/ZigGlobalObject.cpp | 6 +- src/runtime/api/BunObject.rs | 10 ++ test/js/node/process/process.test.js | 151 ++++++++++++++++++ 9 files changed, 270 insertions(+), 170 deletions(-) create mode 100644 bench/snippets/process-env.mjs diff --git a/bench/snippets/process-env.mjs b/bench/snippets/process-env.mjs new file mode 100644 index 000000000000..c76bf8b28d44 --- /dev/null +++ b/bench/snippets/process-env.mjs @@ -0,0 +1,28 @@ +import { bench, run } from "../runner.mjs"; + +// What a program does before the code that is measured here runs: +// a copy for a child process, which reads every variable, +globalThis.childEnv = { ...process.env }; +// a feature flag that is checked often, +for (let n = 0; n < 20_000; n++) globalThis.flag = process.env.NOT_SET_FLAG; +// and dotenv-style writes. +for (const key of ["BENCH_A", "BENCH_B", "BENCH_C"]) process.env[key] = "1"; +delete process.env.BENCH_C; + +const keys = ["HOME", "PATH", "BENCH_A", "NOT_SET_1", "NOT_SET_2", "USER", "NOT_SET_3", "BENCH_B"]; +let i = 0; + +bench("process.env.HOME", () => process.env.HOME); +bench("process.env.NOT_SET", () => process.env.NOT_SET); +bench("process.env[key]", () => process.env[keys[i++ & 7]]); +bench("process.env.BENCH_A = 'value'", () => { + process.env.BENCH_A = "value"; +}); +bench("{ ...process.env }", () => ({ ...process.env })); +bench("Object.keys(process.env)", () => Object.keys(process.env)); +bench("process.argv", () => process.argv); +bench("process.argv[1]", () => process.argv[1]); +bench("process.argv.length", () => process.argv.length); +bench("process.execArgv", () => process.execArgv); + +await run(); diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index efd62913ade4..a4fae131c209 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -3170,105 +3170,42 @@ static JSValue constructExecPath(VM& vm, JSObject* processObject) return JSValue::decode(Bun__Process__getExecPath(globalObject)); } -extern "C" EncodedJSValue Bun__Process__getArgv(JSGlobalObject* lexicalGlobalObject) +static JSValue constructArgv(VM& vm, JSObject* processObject) { - auto* globalObject = defaultGlobalObject(lexicalGlobalObject); - auto* process = globalObject->processObject(); - if (!process) { - return JSValue::encode(jsUndefined()); - } - - return JSValue::encode(process->getArgv(globalObject)); + return JSValue::decode(Bun__Process__createArgv(processObject->globalObject())); } -// get from js -JSC_DEFINE_CUSTOM_GETTER(processArgv, (JSGlobalObject * globalObject, EncodedJSValue thisValue, PropertyName)) +static JSValue constructExecArgv(VM& vm, JSObject* processObject) { - Process* process = getProcessObject(globalObject, JSValue::decode(thisValue)); - if (!process) { - return JSValue::encode(jsUndefined()); - } - - return JSValue::encode(process->getArgv(globalObject)); + return JSValue::decode(Bun__Process__createExecArgv(processObject->globalObject())); } JSValue Process::getArgv(JSGlobalObject* globalObject) { - if (auto argv = m_argv.get()) { - return argv; - } - - JSValue argv = JSValue::decode(Bun__Process__createArgv(globalObject)); - setArgv(globalObject, argv); - return argv; + return get(globalObject, Identifier::fromString(globalObject->vm(), "argv"_s)); } -void Process::setArgv(JSGlobalObject* globalObject, JSValue value) +JSValue Process::getExecArgv(JSGlobalObject* globalObject) { - auto& vm = globalObject->vm(); - m_argv.set(vm, this, value); + return get(globalObject, Identifier::fromString(globalObject->vm(), "execArgv"_s)); } -JSC_DEFINE_CUSTOM_SETTER(setProcessArgv, (JSGlobalObject * globalObject, EncodedJSValue thisValue, EncodedJSValue encodedValue, PropertyName)) +extern "C" EncodedJSValue Bun__Process__getArgv(JSGlobalObject* lexicalGlobalObject) { - Process* process = getProcessObject(globalObject, JSValue::decode(thisValue)); - if (!process) { - return true; - } - - JSValue value = JSValue::decode(encodedValue); - process->setArgv(globalObject, value); - return true; + auto* globalObject = defaultGlobalObject(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); + JSValue argv = globalObject->processObject()->getArgv(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + return JSValue::encode(argv); } extern "C" EncodedJSValue Bun__Process__getExecArgv(JSGlobalObject* lexicalGlobalObject) { auto* globalObject = defaultGlobalObject(lexicalGlobalObject); - auto* process = globalObject->processObject(); - if (!process) { - return JSValue::encode(jsUndefined()); - } - - return JSValue::encode(process->getExecArgv(globalObject)); -} - -JSC_DEFINE_CUSTOM_GETTER(processExecArgv, (JSGlobalObject * globalObject, EncodedJSValue thisValue, PropertyName)) -{ - Process* process = getProcessObject(globalObject, JSValue::decode(thisValue)); - if (!process) { - return JSValue::encode(jsUndefined()); - } - - return JSValue::encode(process->getExecArgv(globalObject)); -} - -JSValue Process::getExecArgv(JSGlobalObject* globalObject) -{ - if (auto argv = m_execArgv.get()) { - return argv; - } - - JSValue argv = JSValue::decode(Bun__Process__createExecArgv(globalObject)); - setExecArgv(globalObject, argv); - return argv; -} - -void Process::setExecArgv(JSGlobalObject* globalObject, JSValue value) -{ - auto& vm = globalObject->vm(); - m_execArgv.set(vm, this, value); -} - -JSC_DEFINE_CUSTOM_SETTER(setProcessExecArgv, (JSGlobalObject * globalObject, EncodedJSValue thisValue, EncodedJSValue encodedValue, PropertyName)) -{ - Process* process = getProcessObject(globalObject, JSValue::decode(thisValue)); - if (!process) { - return true; - } - - JSValue value = JSValue::decode(encodedValue); - process->setExecArgv(globalObject, value); - return true; + auto scope = DECLARE_THROW_SCOPE(globalObject->vm()); + JSValue execArgv = globalObject->processObject()->getExecArgv(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + return JSValue::encode(execArgv); } JSC_DEFINE_CUSTOM_GETTER(processGetEval, (JSGlobalObject * globalObject, EncodedJSValue thisValue, PropertyName)) @@ -3776,8 +3713,6 @@ void Process::visitChildrenImpl(JSCell* cell, Visitor& visitor) visitor.append(thisObject->m_uncaughtExceptionCaptureCallback); visitor.append(thisObject->m_nextTickFunction); visitor.append(thisObject->m_cachedCwd); - visitor.append(thisObject->m_argv); - visitor.append(thisObject->m_execArgv); visitor.append(thisObject->m_onWarning); thisObject->m_cpuUsageStructure.visit(visitor); @@ -5026,7 +4961,7 @@ extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValu loadEnvFile constructLoadEnvFile PropertyCallback finalization constructFinalization PropertyCallback arch constructArch PropertyCallback - argv processArgv CustomAccessor + argv constructArgv PropertyCallback argv0 constructArgv0 PropertyCallback assert Process_functionAssert Function 1 availableMemory Process_availableMemory Function 0 @@ -5045,7 +4980,7 @@ extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValu dlopen Process_functionDlopen Function 1 emitWarning Process_emitWarning Function 1 env constructEnv PropertyCallback - execArgv processExecArgv CustomAccessor + execArgv constructExecArgv PropertyCallback execPath constructExecPath PropertyCallback execve Process_functionExecve Function 3 exit Process_functionExit Function 1 diff --git a/src/jsc/bindings/BunProcess.h b/src/jsc/bindings/BunProcess.h index bec759f7f6a9..c82dc5b0d38a 100644 --- a/src/jsc/bindings/BunProcess.h +++ b/src/jsc/bindings/BunProcess.h @@ -32,8 +32,6 @@ class Process : public WebCore::JSEventEmitter { WriteBarrier m_nextTickFunction; // https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/lib/internal/bootstrap/switches/does_own_process_state.js#L113-L116 WriteBarrier m_cachedCwd; - WriteBarrier m_argv; - WriteBarrier m_execArgv; // The JS warning printer (ProcessObjectInternals createOnWarning), built on the first warning. WriteBarrier m_onWarning; @@ -92,10 +90,7 @@ class Process : public WebCore::JSEventEmitter { void clearCachedCwd() { m_cachedCwd.clear(); } JSValue getArgv(JSGlobalObject* globalObject); - void setArgv(JSGlobalObject* globalObject, JSValue argv); - JSValue getExecArgv(JSGlobalObject* globalObject); - void setExecArgv(JSGlobalObject* globalObject, JSValue execArgv); static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype) diff --git a/src/jsc/bindings/InspectorLifecycleAgent.cpp b/src/jsc/bindings/InspectorLifecycleAgent.cpp index dc32891c9187..198e8ffefb16 100644 --- a/src/jsc/bindings/InspectorLifecycleAgent.cpp +++ b/src/jsc/bindings/InspectorLifecycleAgent.cpp @@ -172,9 +172,10 @@ Protocol::ErrorStringOr InspectorLifecycleAgent::getModuleGraph() Ref> argv = JSON::ArrayOf::create(); { - auto* array = uncheckedDowncast(process->getArgv(global)); + JSC::JSValue argvValue = process->getArgv(global); RETURN_IF_EXCEPTION(scope, fail("Failed to get argv"_s)); - for (size_t i = 0, length = array->length(); i < length; i++) { + auto* array = dynamicDowncast(argvValue); + for (size_t i = 0, length = array ? array->length() : 0; i < length; i++) { auto value = array->getIndex(global, i); RETURN_IF_EXCEPTION(scope, fail("Failed to get value at index"_s)); auto string = value.toWTFString(global); diff --git a/src/jsc/bindings/JSEnvironmentVariableMap.cpp b/src/jsc/bindings/JSEnvironmentVariableMap.cpp index e3d6192daed8..681f1bfd1a06 100644 --- a/src/jsc/bindings/JSEnvironmentVariableMap.cpp +++ b/src/jsc/bindings/JSEnvironmentVariableMap.cpp @@ -36,6 +36,7 @@ using namespace JSC; extern "C" size_t Bun__getEnvCount(JSGlobalObject* globalObject, void** list_ptr); extern "C" size_t Bun__getEnvKey(void* list, size_t index, unsigned char** out); +extern "C" void Bun__getEnvValueAt(JSGlobalObject* globalObject, size_t index, EncodedSlice* value); extern "C" bool Bun__getEnvValue(JSGlobalObject* globalObject, const EncodedSlice* name, EncodedSlice* value); extern "C" void Bun__setEnvValue(JSGlobalObject* globalObject, const BunString* name, const BunString* value); @@ -216,7 +217,18 @@ bool JSEnvironmentVariableMap::put(JSCell* cell, JSGlobalObject* globalObject, P static_cast(cell)->putDirect(vm, propertyName, string, 0); return true; } - RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, string, slot)); + // Not `slot`: an inline cache would store `value` uncoerced. PutById: a dictionary after 512 properties, not 128. + PutPropertySlot ownSlot(cell, slot.isStrictMode(), PutPropertySlot::PutById); + RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, string, ownSlot)); +} + +void JSEnvironmentVariableMap::putInitialValue(JSGlobalObject* globalObject, const Identifier& name, JSValue value) +{ + if (auto index = parseIndex(name)) [[unlikely]] { + putDirectIndex(globalObject, *index, value, 0, PutDirectIndexLikePutDirect); + return; + } + putDirectWithoutTransition(globalObject->vm(), name, value, 0); } bool JSEnvironmentVariableMap::putByIndex(JSCell* cell, JSGlobalObject* globalObject, unsigned index, JSValue value, bool shouldThrow) @@ -257,30 +269,6 @@ bool JSEnvironmentVariableMap::defineOwnProperty(JSObject* object, JSGlobalObjec RELEASE_AND_RETURN(scope, put(object, globalObject, propertyName, descriptor.value(), slot)); } -JSC_DEFINE_CUSTOM_GETTER(jsGetterEnvironmentVariable, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, PropertyName propertyName)) -{ - VM& vm = globalObject->vm(); - auto scope = DECLARE_THROW_SCOPE(vm); - - auto* thisObject = dynamicDowncast(JSValue::decode(thisValue)); - if (!thisObject) [[unlikely]] - return JSValue::encode(jsUndefined()); - - EncodedSlice name = toEncodedSlice(propertyName.publicName()); - EncodedSlice value = { nullptr, 0 }; - - if (name.len == 0) [[unlikely]] - return JSValue::encode(jsUndefined()); - - if (!Bun__getEnvValue(globalObject, &name, &value)) { - return JSValue::encode(jsUndefined()); - } - - JSValue result = jsString(vm, Zig::toStringCopy(value)); - thisObject->putDirect(vm, propertyName, result, 0); - return JSValue::encode(result); -} - JSC_DEFINE_CUSTOM_GETTER(jsTimeZoneEnvironmentVariableGetter, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, PropertyName propertyName)) { VM& vm = globalObject->vm(); @@ -305,10 +293,7 @@ JSC_DEFINE_CUSTOM_GETTER(jsTimeZoneEnvironmentVariableGetter, (JSGlobalObject * return JSValue::encode(jsUndefined()); } - JSValue out = jsString(vm, Zig::toStringCopy(value)); - thisObject->putDirect(vm, clientData->builtinNames().dataPrivateName(), out, 0); - - return JSValue::encode(out); + return JSValue::encode(jsString(vm, Zig::toStringCopy(value))); } // Store-only: the TZ side effect fires from put() / jsProcessEnvCoerceForWrite on every @@ -994,17 +979,28 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) RETURN_IF_EXCEPTION(scope, {}); #else auto* structure = JSEnvironmentVariableMap::createStructure(vm, globalObject, globalObject->objectPrototype()); - JSC::JSObject* object = JSEnvironmentVariableMap::create(vm, structure); + auto* object = JSEnvironmentVariableMap::create(vm, structure); #endif static NeverDestroyed TZ = MAKE_STATIC_STRING_IMPL("TZ"); String NODE_TLS_REJECT_UNAUTHORIZED = String("NODE_TLS_REJECT_UNAUTHORIZED"_s); String BUN_CONFIG_VERBOSE_FETCH = String("BUN_CONFIG_VERBOSE_FETCH"_s); - bool hasTZ = false; + JSString* tz = nullptr; bool hasNodeTLSRejectUnauthorized = false; bool hasBunConfigVerboseFetch = false; - auto* cached_getter_setter = JSC::CustomGetterSetter::create(vm, jsGetterEnvironmentVariable, nullptr); + auto valueAt = [&](size_t index) { + EncodedSlice value = { nullptr, 0 }; + Bun__getEnvValueAt(globalObject, index, &value); + return jsString(vm, Zig::toStringCopy(value)); + }; + auto putValue = [&](const Identifier& name, JSValue value) { +#if OS(WINDOWS) + object->putDirectMayBeIndex(globalObject, name, value); +#else + object->putInitialValue(globalObject, name, value); +#endif + }; for (size_t i = 0; i < count; i++) { unsigned char* chars; @@ -1015,7 +1011,7 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) keyArray->putByIndexInline(globalObject, (unsigned)i, jsString(vm, name), false); #endif if (name == TZ) { - hasTZ = true; + tz = valueAt(i); continue; } if (name == NODE_TLS_REJECT_UNAUTHORIZED) { @@ -1033,53 +1029,30 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) String idName = name; #endif Identifier identifier = Identifier::fromString(vm, idName); - - // CustomGetterSetter doesn't support indexed properties yet. - // This causes strange issues when the environment variable name is an integer. - if (chars[0] >= '0' && chars[0] <= '9') [[unlikely]] { - if (auto index = parseIndex(identifier)) { - EncodedSlice valueString = { nullptr, 0 }; - EncodedSlice nameStr = toEncodedSlice(name); - if (Bun__getEnvValue(globalObject, &nameStr, &valueString)) { - JSValue value = jsString(vm, Zig::toStringCopy(valueString)); - RETURN_IF_EXCEPTION(scope, {}); - object->putDirectIndex(globalObject, *index, value, 0, PutDirectIndexLikePutDirect); - RETURN_IF_EXCEPTION(scope, {}); - } - continue; - } - } - - // JSC::PropertyAttribute::CustomValue calls the getter ONCE (the first - // time) and then sets it onto the object, subsequent calls to the - // getter will not go through the getter and instead will just do the - // property lookup. - object->putDirectCustomAccessor(vm, identifier, cached_getter_setter, JSC::PropertyAttribute::CustomValue | 0); - } - - unsigned int TZAttrs = JSC::PropertyAttribute::CustomAccessor | 0; - if (!hasTZ) { - TZAttrs |= JSC::PropertyAttribute::DontEnum; - } - object->putDirectCustomAccessor( - vm, - Identifier::fromString(vm, TZ), JSC::CustomGetterSetter::create(vm, jsTimeZoneEnvironmentVariableGetter, jsTimeZoneEnvironmentVariableSetter), TZAttrs); - - unsigned int NODE_TLS_REJECT_UNAUTHORIZED_Attrs = JSC::PropertyAttribute::CustomAccessor | 0; - if (!hasNodeTLSRejectUnauthorized) { - NODE_TLS_REJECT_UNAUTHORIZED_Attrs |= JSC::PropertyAttribute::DontEnum; + // Two names that are not valid UTF-8 can decode to the same string. + if (!idName.is8Bit() && isValidOffset(object->getDirectOffset(vm, identifier))) [[unlikely]] + continue; + putValue(identifier, valueAt(i)); + RETURN_IF_EXCEPTION(scope, {}); } - object->putDirectCustomAccessor( - vm, - Identifier::fromString(vm, NODE_TLS_REJECT_UNAUTHORIZED), JSC::CustomGetterSetter::create(vm, jsNodeTLSRejectUnauthorizedGetter, jsNodeTLSRejectUnauthorizedSetter), NODE_TLS_REJECT_UNAUTHORIZED_Attrs); - unsigned int BUN_CONFIG_VERBOSE_FETCH_Attrs = JSC::PropertyAttribute::CustomAccessor | 0; - if (!hasBunConfigVerboseFetch) { - BUN_CONFIG_VERBOSE_FETCH_Attrs |= JSC::PropertyAttribute::DontEnum; - } - object->putDirectCustomAccessor( - vm, - Identifier::fromString(vm, BUN_CONFIG_VERBOSE_FETCH), JSC::CustomGetterSetter::create(vm, jsBunConfigVerboseFetchGetter, jsBunConfigVerboseFetchSetter), BUN_CONFIG_VERBOSE_FETCH_Attrs); + auto putAccessor = [&](const String& name, GetValueFunc getter, PutValueFunc setter, bool isSet) { + unsigned attributes = JSC::PropertyAttribute::CustomAccessor | 0; + if (!isSet) + attributes |= JSC::PropertyAttribute::DontEnum; + auto* accessor = JSC::CustomGetterSetter::create(vm, getter, setter); +#if OS(WINDOWS) + object->putDirectCustomAccessor(vm, Identifier::fromString(vm, name), accessor, attributes); +#else + object->putDirectCustomGetterSetterWithoutTransition(vm, Identifier::fromString(vm, name), accessor, attributes); +#endif + }; + putAccessor(TZ, jsTimeZoneEnvironmentVariableGetter, jsTimeZoneEnvironmentVariableSetter, !!tz); + // The getter must not add this on the first read: that would change the structure. + if (tz && tz->length()) + putValue(WebCore::clientData(vm)->builtinNames().dataPrivateName(), tz); + putAccessor(NODE_TLS_REJECT_UNAUTHORIZED, jsNodeTLSRejectUnauthorizedGetter, jsNodeTLSRejectUnauthorizedSetter, hasNodeTLSRejectUnauthorized); + putAccessor(BUN_CONFIG_VERBOSE_FETCH, jsBunConfigVerboseFetchGetter, jsBunConfigVerboseFetchSetter, hasBunConfigVerboseFetch); #if OS(WINDOWS) auto editWindowsEnvVar = JSC::JSFunction::create(vm, globalObject, 0, String("editWindowsEnvVar"_s), jsEditWindowsEnvVar, ImplementationVisibility::Public); diff --git a/src/jsc/bindings/JSEnvironmentVariableMap.h b/src/jsc/bindings/JSEnvironmentVariableMap.h index c83ce7134246..4e684f7a4da8 100644 --- a/src/jsc/bindings/JSEnvironmentVariableMap.h +++ b/src/jsc/bindings/JSEnvironmentVariableMap.h @@ -41,6 +41,9 @@ class JSEnvironmentVariableMap final : public JSC::JSNonFinalObject { return Bun::createClassStructure(vm, globalObject, prototype, JSC::TypeInfo(JSC::ObjectType, StructureFlags), info()); } + // Fills in a map nothing has read yet, without a Structure per variable. `name` must not be on the map. + void putInitialValue(JSC::JSGlobalObject*, const JSC::Identifier& name, JSC::JSValue); + static bool put(JSC::JSCell*, JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&); static bool putByIndex(JSC::JSCell*, JSC::JSGlobalObject*, unsigned, JSC::JSValue, bool shouldThrow); static bool defineOwnProperty(JSC::JSObject*, JSC::JSGlobalObject*, JSC::PropertyName, const JSC::PropertyDescriptor&, bool shouldThrow); diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 73443d4f0cc5..3fa56f8b2a90 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -616,14 +616,18 @@ extern "C" JSC::JSGlobalObject* Zig__GlobalObject__create(void* console_client, // worker coerce to string, reject symbol keys, and validate // defineProperty like Node's EnvSetter/EnvDefiner. auto* envStructure = Bun::JSEnvironmentVariableMap::createStructure(vm, globalObject, globalObject->objectPrototype()); - JSC::JSObject* env = Bun::JSEnvironmentVariableMap::create(vm, envStructure); + auto* env = Bun::JSEnvironmentVariableMap::create(vm, envStructure); #endif size_t i = 0; for (auto k : map) { // Numeric env keys hit putDirectIndex → defineOwnProperty (declares a // ThrowScope). Seeded values are JSStrings, so this throws only on OOM // or under a termination already requested for this starting worker. +#if OS(WINDOWS) env->putDirectMayBeIndex(globalObject, JSC::Identifier::fromString(vm, WTF::move(k.key)), strings.at(i++)); +#else + env->putInitialValue(globalObject, JSC::Identifier::fromString(vm, WTF::move(k.key)), strings.at(i++)); +#endif if (scope.exception()) [[unlikely]] break; } diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index d5dea22f877e..bb6633b56094 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1996,6 +1996,16 @@ pub(crate) mod environment_variables { item.len() } + #[unsafe(no_mangle)] + extern "C" fn Bun__getEnvValueAt<'a>( + global_object: &'a JSGlobalObject, + i: usize, + value: &mut core::mem::MaybeUninit>, + ) { + let env = global_object.bun_vm().env_loader(); + value.write(EncodedSlice::from_bytes(&env.map.map.values()[i].value)); + } + #[unsafe(no_mangle)] extern "C" fn Bun__getEnvValue<'a>( global_object: &'a JSGlobalObject, diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 90ca9bcac27f..251c9d74c8fe 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -5,6 +5,7 @@ import { describe, expect, it } from "bun:test"; import { familySync } from "detect-libc"; import { bunEnv, bunExe, isASAN, isDebug, isMacOS, isWindows, tempDir, tmpdirSync } from "harness"; import { basename, join, resolve } from "path"; +import { parseArgs } from "util"; import { getHeapStatistics } from "v8"; const process_sleep = resolve(import.meta.dir, "process-sleep.js"); @@ -520,6 +521,121 @@ it("process.env reads are never stale after a write (JIT inline-cache soundness) }); }); +// On Windows process.env is a Proxy, which has no structure of its own to look at. +it.skipIf(isWindows)("reading process.env does not change its structure", async () => { + // JSC turns an object into a dictionary, which inline caches give up on, after 128 transitions. + const env = { ...bunEnv }; + for (let i = 0; i < 200; i++) env["STRUCTURE_TEST_" + i] = "value " + i; + using dir = tempDir("process-env-structure", { + "index.mjs": ` + import { describe } from "bun:jsc"; + import { Worker, isMainThread, parentPort } from "node:worker_threads"; + + function probe() { + const shape = () => describe(process.env).match(/StructureID: \\d+|Dictionary/g).join(" "); + const before = shape(); + let read = 0; + for (const key in process.env) read += typeof process.env[key] === "string"; + read += Object.keys({ ...process.env }).length; + for (let i = 0; i < 20000; i++) read += process.env.STRUCTURE_TEST_MISSING !== undefined; + return { read: read >= 400, dictionary: before.includes("Dictionary"), changed: shape() !== before }; + } + + if (isMainThread) { + const main = probe(); + const worker = await new Promise((resolve, reject) => { + new Worker(import.meta.filename).once("message", resolve).once("error", reject); + }); + console.log(JSON.stringify({ main, worker })); + } else { + parentPort.postMessage(probe()); + } + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.mjs"], + env, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const expected = { read: true, dictionary: false, changed: false }; + expect({ out: JSON.parse(stdout || "null"), stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ + out: { main: expected, worker: expected }, + stderr: "", + exitCode: 0, + }); +}); + +// The values are read through getOwnPropertyDescriptor so that no inline cache is involved in the check. +const countRawWrites = body => ` + ${body} + let raw = 0; + for (let i = 0; i < 300000; i++) { + write(i); + if (typeof Object.getOwnPropertyDescriptor(process.env, "HOT_WRITE").value !== "string") raw++; + } + console.log(raw); +`; + +it("process.env coerces every write from a hot site to a string", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", countRawWrites(`function write(value) { process.env.HOT_WRITE = value; }`)], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ + stdout: "0\n", + stderr: "", + exitCode: 0, + }); +}); + +// PutByStatus::computeFor(StructureSet) in JSC does not look at OverridesPut, so the DFG stores the value directly. +it.todo("process.env coerces every write from a hot site that also reads the variable", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + countRawWrites(`function write(value) { process.env.HOT_WRITE = value; return process.env.HOT_WRITE; }`), + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ + stdout: "0\n", + stderr: "", + exitCode: 0, + }); +}); + +// Windows environment blocks are UTF-16. +it.skipIf(isWindows)("process.env has the value of a variable whose name is not valid UTF-8", async () => { + await using proc = Bun.spawn({ + cmd: [ + "sh", + "-c", + `exec env "$(printf 'BAD\\377')=first" "$(printf 'BAD\\376')=second" "$0" -e 'console.log(JSON.stringify(Object.entries(process.env).filter(([k]) => k.startsWith("BAD"))))'`, + bunExe(), + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Both names decode to "BAD\ufffd". + expect({ out: JSON.parse(stdout || "null"), stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ + out: [["BAD\ufffd", expect.stringMatching(/^(first|second)$/)]], + stderr: "", + exitCode: 0, + }); +}); + const MIN_ICU_VERSIONS_BY_PLATFORM_ARCH = { "darwin-x64": "70.1", "darwin-arm64": "72.1", @@ -758,6 +874,41 @@ it("process.argv in testing", () => { expect(process.argv).toBe(process.argv); }); +it("process.argv and process.execArgv are data properties", () => { + for (const key of ["argv", "execArgv"]) { + expect(Object.getOwnPropertyDescriptor(process, key)).toEqual({ + value: process[key], + writable: true, + enumerable: true, + configurable: true, + }); + } +}); + +it("util.parseArgs reads process.argv the way JavaScript does", () => { + const original = Object.getOwnPropertyDescriptor(process, "argv"); + const options = { a: { type: "boolean" }, b: { type: "boolean" } }; + try { + process.argv = ["bun", "script.js", "-a"]; + expect(parseArgs({ options }).values).toEqual({ a: true }); + Object.defineProperty(process, "argv", { get: () => ["bun", "script.js", "-b"], configurable: true }); + expect(parseArgs({ options }).values).toEqual({ b: true }); + Object.defineProperty(process, "argv", { + get() { + throw new Error("argv getter"); + }, + configurable: true, + }); + expect(() => parseArgs({ options })).toThrow("argv getter"); + for (const notAnArray of [1, {}]) { + Object.defineProperty(process, "argv", { value: notAnArray, configurable: true }); + expect(parseArgs({ options }).values).toEqual({}); + } + } finally { + Object.defineProperty(process, "argv", original); + } +}); + describe("process.exitCode", () => { it("validates int", () => { expect(() => (process.exitCode = "potato")).toThrow( From 1d95a0b5e47a214f9ad00c0c349e2f239617eb1d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:46:50 +0000 Subject: [PATCH 2/4] process.test.js: make the new process.env tests concurrent, pin the duplicate name, cover Bun.argv - The hot-write tests run 10,000 writes, not 300,000. They turn the concurrent JIT off and lower the FTL threshold, so write() is in the FTL within the first 400 calls. A debug build needed 8.7 s for the old loop and the test timed out there. - The spawn tests for process.env are concurrent. - Two names that are not valid UTF-8 and decode to the same string: the test expects the value of the first one in the environment. - New test: Bun.argv keeps no value when the process.argv getter throws. The next read calls the getter again. --- test/js/node/process/process.test.js | 57 +++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 251c9d74c8fe..3803f0b8c71c 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -522,7 +522,7 @@ it("process.env reads are never stale after a write (JIT inline-cache soundness) }); // On Windows process.env is a Proxy, which has no structure of its own to look at. -it.skipIf(isWindows)("reading process.env does not change its structure", async () => { +it.concurrent.skipIf(isWindows)("reading process.env does not change its structure", async () => { // JSC turns an object into a dictionary, which inline caches give up on, after 128 transitions. const env = { ...bunEnv }; for (let i = 0; i < 200; i++) env["STRUCTURE_TEST_" + i] = "value " + i; @@ -572,17 +572,19 @@ it.skipIf(isWindows)("reading process.env does not change its structure", async const countRawWrites = body => ` ${body} let raw = 0; - for (let i = 0; i < 300000; i++) { + for (let i = 0; i < 10000; i++) { write(i); if (typeof Object.getOwnPropertyDescriptor(process.env, "HOT_WRITE").value !== "string") raw++; } console.log(raw); `; +// The concurrent JIT is off and the FTL threshold is low: write() reaches every tier early in the loop, in a debug build too. +const hotWriteEnv = { ...bunEnv, BUN_JSC_useConcurrentJIT: "0", BUN_JSC_thresholdForFTLOptimizeAfterWarmUp: "1000" }; -it("process.env coerces every write from a hot site to a string", async () => { +it.concurrent("process.env coerces every write from a hot site to a string", async () => { await using proc = Bun.spawn({ cmd: [bunExe(), "-e", countRawWrites(`function write(value) { process.env.HOT_WRITE = value; }`)], - env: bunEnv, + env: hotWriteEnv, stdout: "pipe", stderr: "pipe", }); @@ -595,14 +597,14 @@ it("process.env coerces every write from a hot site to a string", async () => { }); // PutByStatus::computeFor(StructureSet) in JSC does not look at OverridesPut, so the DFG stores the value directly. -it.todo("process.env coerces every write from a hot site that also reads the variable", async () => { +it.concurrent.todo("process.env coerces every write from a hot site that also reads the variable", async () => { await using proc = Bun.spawn({ cmd: [ bunExe(), "-e", countRawWrites(`function write(value) { process.env.HOT_WRITE = value; return process.env.HOT_WRITE; }`), ], - env: bunEnv, + env: hotWriteEnv, stdout: "pipe", stderr: "pipe", }); @@ -615,7 +617,7 @@ it.todo("process.env coerces every write from a hot site that also reads the var }); // Windows environment blocks are UTF-16. -it.skipIf(isWindows)("process.env has the value of a variable whose name is not valid UTF-8", async () => { +it.concurrent.skipIf(isWindows)("process.env has the value of a variable whose name is not valid UTF-8", async () => { await using proc = Bun.spawn({ cmd: [ "sh", @@ -628,9 +630,9 @@ it.skipIf(isWindows)("process.env has the value of a variable whose name is not stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - // Both names decode to "BAD\ufffd". + // Both names decode to "BAD\ufffd". The one that comes first in the environment keeps the name. expect({ out: JSON.parse(stdout || "null"), stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ - out: [["BAD\ufffd", expect.stringMatching(/^(first|second)$/)]], + out: [["BAD\ufffd", "first"]], stderr: "", exitCode: 0, }); @@ -909,6 +911,43 @@ it("util.parseArgs reads process.argv the way JavaScript does", () => { } }); +// Bun.argv takes its value from process.argv on the first read. The child has not read it yet. +it.concurrent("Bun.argv keeps no value when the process.argv getter throws", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + Object.defineProperty(process, "argv", { + get() { + throw new Error("argv getter"); + }, + configurable: true, + }); + const thrown = []; + for (let i = 0; i < 2; i++) { + try { + Bun.argv; + } catch (error) { + thrown.push(error.message); + } + } + Object.defineProperty(process, "argv", { value: ["a", "b"], configurable: true }); + console.log(JSON.stringify({ thrown, argv: Bun.argv, same: Bun.argv === process.argv })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ out: JSON.parse(stdout || "null"), stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ + out: { thrown: ["argv getter", "argv getter"], argv: ["a", "b"], same: true }, + stderr: "", + exitCode: 0, + }); +}); + describe("process.exitCode", () => { it("validates int", () => { expect(() => (process.exitCode = "potato")).toThrow( From ba9ff1ca5aeb929fcc890da3eb469ca077d8c2f2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:59:30 +0000 Subject: [PATCH 3/4] process.env: use the PutById slot for every store that can add a property put() stored TZ, NODE_TLS_REJECT_UNAUTHORIZED and the proxy variables with putDirect and no slot. With more than 128 properties on process.env, JSC makes the object a dictionary when such a store adds a property. The JIT flattens a dictionary one time only. After the next such store, a read of a variable that is not set is not cached again: 3,400 to 4,000 ns per read against about 70 ns (debug build, 200 variables). These stores now use the PutById slot that put() has for every other name, so the limit is 512 properties for all of them. The custom setters add their private value the same way, and get the same slot. On POSIX only the BUN_CONFIG_VERBOSE_FETCH setter runs for process.env: put() handles the other two names. --- src/jsc/bindings/JSEnvironmentVariableMap.cpp | 26 +++++++--- test/js/node/process/process.test.js | 51 +++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/jsc/bindings/JSEnvironmentVariableMap.cpp b/src/jsc/bindings/JSEnvironmentVariableMap.cpp index 681f1bfd1a06..22ace61c9724 100644 --- a/src/jsc/bindings/JSEnvironmentVariableMap.cpp +++ b/src/jsc/bindings/JSEnvironmentVariableMap.cpp @@ -196,29 +196,31 @@ bool JSEnvironmentVariableMap::put(JSCell* cell, JSGlobalObject* globalObject, P JSString* string = coerceEnvValue(globalObject, scope, value); RETURN_IF_EXCEPTION(scope, false); + // Not `slot`: an inline cache would store `value` uncoerced. PutById: a dictionary after 512 properties, not 128. + PutPropertySlot ownSlot(cell, slot.isStrictMode(), PutPropertySlot::PutById); + auto* thisObject = static_cast(cell); + // Node's RealEnvStore::Set name-matches TZ on every write, so delete-then-set still // updates Date caches. putDirect bypasses the accessor so the side effect fires once. if (uid && WTF::equal(uid, "TZ"_s)) [[unlikely]] { applyTimeZoneEnvValue(globalObject, string); RETURN_IF_EXCEPTION(scope, false); - static_cast(cell)->putDirect(vm, propertyName, string, 0); + thisObject->putDirect(vm, propertyName, string, 0, ownSlot); return true; } if (uid && WTF::equal(uid, "NODE_TLS_REJECT_UNAUTHORIZED"_s)) [[unlikely]] { applyTLSRejectEnvValue(globalObject, string); RETURN_IF_EXCEPTION(scope, false); - static_cast(cell)->putDirect(vm, propertyName, string, 0); + thisObject->putDirect(vm, propertyName, string, 0, ownSlot); return true; } // fetch() reads the proxy variables from the native env map. if (isProxyEnvVarName(vm, uid)) [[unlikely]] { setNativeEnvValue(globalObject, String(uid), string); RETURN_IF_EXCEPTION(scope, false); - static_cast(cell)->putDirect(vm, propertyName, string, 0); + thisObject->putDirect(vm, propertyName, string, 0, ownSlot); return true; } - // Not `slot`: an inline cache would store `value` uncoerced. PutById: a dictionary after 512 properties, not 128. - PutPropertySlot ownSlot(cell, slot.isStrictMode(), PutPropertySlot::PutById); RELEASE_AND_RETURN(scope, Base::put(cell, globalObject, propertyName, string, ownSlot)); } @@ -296,6 +298,14 @@ JSC_DEFINE_CUSTOM_GETTER(jsTimeZoneEnvironmentVariableGetter, (JSGlobalObject * return JSValue::encode(jsString(vm, Zig::toStringCopy(value))); } +// The setters of the three accessors keep the written value under a private name. +// PutById, as in put(): the first write adds a property. +static void putPrivateValue(VM& vm, JSObject* object, const Identifier& privateName, JSValue value) +{ + PutPropertySlot slot(object, false, PutPropertySlot::PutById); + object->putDirect(vm, privateName, value, 0, slot); +} + // Store-only: the TZ side effect fires from put() / jsProcessEnvCoerceForWrite on every // write. Firing here too would double-apply on Windows (writeEnvVar already ran it). JSC_DEFINE_CUSTOM_SETTER(jsTimeZoneEnvironmentVariableSetter, (JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::EncodedJSValue value, PropertyName propertyName)) @@ -305,7 +315,7 @@ JSC_DEFINE_CUSTOM_SETTER(jsTimeZoneEnvironmentVariableSetter, (JSGlobalObject * if (!object) return false; auto* clientData = WebCore::clientData(vm); - object->putDirect(vm, clientData->builtinNames().dataPrivateName(), JSValue::decode(value), 0); + putPrivateValue(vm, object, clientData->builtinNames().dataPrivateName(), JSValue::decode(value)); return true; } @@ -398,7 +408,7 @@ JSC_DEFINE_CUSTOM_SETTER(jsNodeTLSRejectUnauthorizedSetter, (JSGlobalObject * gl applyTLSRejectFromString(globalObject, str); const auto& privateName = NODE_TLS_REJECT_UNAUTHORIZED_PRIVATE_PROPERTY(vm); - object->putDirect(vm, privateName, JSValue::decode(value), 0); + putPrivateValue(vm, object, privateName, JSValue::decode(value)); // TODO: this is an assertion failure // Recreate this because the property visibility needs to be set correctly @@ -446,7 +456,7 @@ JSC_DEFINE_CUSTOM_SETTER(jsBunConfigVerboseFetchSetter, (JSGlobalObject * global applyVerboseFetchFromString(globalObject, str); const auto& privateName = BUN_CONFIG_VERBOSE_FETCH_PRIVATE_PROPERTY(vm); - object->putDirect(vm, privateName, JSValue::decode(value), 0); + putPrivateValue(vm, object, privateName, JSValue::decode(value)); // TODO: this is an assertion failure // Recreate this because the property visibility needs to be set correctly diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index 3803f0b8c71c..66c0948b664d 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -568,6 +568,57 @@ it.concurrent.skipIf(isWindows)("reading process.env does not change its structu }); }); +// With more than 128 properties, JSC makes an object a dictionary when native code adds one more with a default slot. +it.concurrent.skipIf(isWindows)("a new variable does not make process.env a dictionary", async () => { + // TZ, NODE_TLS_REJECT_UNAUTHORIZED and BUN_CONFIG_VERBOSE_FETCH are accessors on the main thread only. + const names = ["HTTP_PROXY", "https_proxy", "NODE_TLS_REJECT_UNAUTHORIZED", "BUN_CONFIG_VERBOSE_FETCH", "NEW"]; + const env = { ...bunEnv }; + for (const name of names) delete env[name]; + for (let i = Object.keys(env).length; i < 200; i++) env["STRUCTURE_TEST_" + i] = "value " + i; + using dir = tempDir("process-env-new-variable", { + "index.mjs": ` + import { describe } from "bun:jsc"; + import { Worker, isMainThread, parentPort } from "node:worker_threads"; + + // The writes after which process.env is a dictionary. + function probe() { + const dictionary = []; + const write = name => { + process.env[name] = "x"; + if (describe(process.env).includes("Dictionary")) dictionary.push(name); + }; + for (const name of ${JSON.stringify(names)}) write(name); + delete process.env.TZ; + write("TZ"); + return dictionary; + } + + if (isMainThread) { + // The worker starts with a copy of the variables, so it runs before the main thread adds any. + const worker = await new Promise((resolve, reject) => { + new Worker(import.meta.filename).once("message", resolve).once("error", reject); + }); + console.log(JSON.stringify({ main: probe(), worker })); + } else { + parentPort.postMessage(probe()); + } + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.mjs"], + env, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ out: JSON.parse(stdout || "null"), stderr: exitCode === 0 ? "" : stderr, exitCode }).toEqual({ + out: { main: [], worker: [] }, + stderr: "", + exitCode: 0, + }); +}); + // The values are read through getOwnPropertyDescriptor so that no inline cache is involved in the check. const countRawWrites = body => ` ${body} From e6621c3268c6b651b26805ff6980cb2b286b1cd4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 17:13:23 +0000 Subject: [PATCH 4/4] ci: retrigger