Skip to content

Commit

Permalink
esm: improve getFormatOfExtensionlessFile speed
Browse files Browse the repository at this point in the history
PR-URL: #49965
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
anonrig authored and targos committed Nov 11, 2023
1 parent f63a92b commit 1e762dd
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 18 deletions.
29 changes: 11 additions & 18 deletions lib/internal/modules/esm/formats.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
'use strict';

const {
RegExpPrototypeExec,
Uint8Array,
} = primordials;
const { RegExpPrototypeExec } = primordials;
const { getOptionValue } = require('internal/options');

const { closeSync, openSync, readSync } = require('fs');
const { getValidatedPath } = require('internal/fs/utils');
const pathModule = require('path');
const fsBindings = internalBinding('fs');
const { fs: fsConstants } = internalBinding('constants');

const experimentalWasmModules = getOptionValue('--experimental-wasm-modules');

Expand Down Expand Up @@ -47,20 +46,14 @@ function mimeToFormat(mime) {
function getFormatOfExtensionlessFile(url) {
if (!experimentalWasmModules) { return 'module'; }

const magic = new Uint8Array(4);
let fd;
try {
// TODO(@anonrig): Optimize the following by having a single C++ call
fd = openSync(url);
readSync(fd, magic, 0, 4); // Only read the first four bytes
if (magic[0] === 0x00 && magic[1] === 0x61 && magic[2] === 0x73 && magic[3] === 0x6d) {
const path = pathModule.toNamespacedPath(getValidatedPath(url));

switch (fsBindings.getFormatOfExtensionlessFile(path)) {
case fsConstants.EXTENSIONLESS_FORMAT_WASM:
return 'wasm';
}
} finally {
if (fd !== undefined) { closeSync(fd); }
default:
return 'module';
}

return 'module';
}

module.exports = {
Expand Down
4 changes: 4 additions & 0 deletions src/node_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,10 @@ void DefineSystemConstants(Local<Object> target) {
NODE_DEFINE_CONSTANT(target, UV_DIRENT_CHAR);
NODE_DEFINE_CONSTANT(target, UV_DIRENT_BLOCK);

// Define module specific constants
NODE_DEFINE_CONSTANT(target, EXTENSIONLESS_FORMAT_JAVASCRIPT);
NODE_DEFINE_CONSTANT(target, EXTENSIONLESS_FORMAT_WASM);

NODE_DEFINE_CONSTANT(target, S_IFMT);
NODE_DEFINE_CONSTANT(target, S_IFREG);
NODE_DEFINE_CONSTANT(target, S_IFDIR);
Expand Down
3 changes: 3 additions & 0 deletions src/node_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
#include "node.h"
#include "v8.h"

#define EXTENSIONLESS_FORMAT_JAVASCRIPT (0)
#define EXTENSIONLESS_FORMAT_WASM (1)

#if HAVE_OPENSSL

#ifndef RSA_PSS_SALTLEN_DIGEST
Expand Down
50 changes: 50 additions & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2832,6 +2832,51 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
}
}

static void GetFormatOfExtensionlessFile(
const FunctionCallbackInfo<Value>& args) {
CHECK_EQ(args.Length(), 1);
CHECK(args[0]->IsString());

Environment* env = Environment::GetCurrent(args);
node::Utf8Value input(args.GetIsolate(), args[0]);

THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemRead, input.ToStringView());

uv_fs_t req;
FS_SYNC_TRACE_BEGIN(open)
uv_file file = uv_fs_open(nullptr, &req, input.out(), O_RDONLY, 0, nullptr);
FS_SYNC_TRACE_END(open);

if (req.result < 0) {
return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT);
}

auto cleanup = OnScopeLeave([&req, &file]() {
FS_SYNC_TRACE_BEGIN(close);
CHECK_EQ(0, uv_fs_close(nullptr, &req, file, nullptr));
FS_SYNC_TRACE_END(close);
uv_fs_req_cleanup(&req);
});

char buffer[4];
uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));
int err = uv_fs_read(nullptr, &req, file, &buf, 1, 0, nullptr);

if (err < 0) {
return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT);
}

// We do this by taking advantage of the fact that all Wasm files start with
// the header `0x00 0x61 0x73 0x6d`
if (buffer[0] == 0x00 && buffer[1] == 0x61 && buffer[2] == 0x73 &&
buffer[3] == 0x6d) {
return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_WASM);
}

return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT);
}

static bool FileURLToPath(
Environment* env,
const ada::url_aggregator& file_url,
Expand Down Expand Up @@ -3244,6 +3289,10 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();

SetMethod(isolate,
target,
"getFormatOfExtensionlessFile",
GetFormatOfExtensionlessFile);
SetMethod(isolate, target, "access", Access);
SetMethod(isolate, target, "close", Close);
SetMethod(isolate, target, "existsSync", ExistsSync);
Expand Down Expand Up @@ -3364,6 +3413,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
StatWatcher::RegisterExternalReferences(registry);
BindingData::RegisterExternalReferences(registry);

registry->Register(GetFormatOfExtensionlessFile);
registry->Register(Close);
registry->Register(ExistsSync);
registry->Register(Open);
Expand Down
2 changes: 2 additions & 0 deletions typings/internalBinding/constants.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ export interface ConstantsBinding {
COPYFILE_FICLONE: 2;
UV_FS_COPYFILE_FICLONE_FORCE: 4;
COPYFILE_FICLONE_FORCE: 4;
EXTENSIONLESS_FORMAT_JAVASCRIPT: 0;
EXTENSIONLESS_FORMAT_WASM: 1;
};
crypto: {
OPENSSL_VERSION_NUMBER: 269488319;
Expand Down
6 changes: 6 additions & 0 deletions typings/internalBinding/fs.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { ConstantsBinding } from './constants';

declare namespace InternalFSBinding {
class FSReqCallback<ResultType = unknown> {
constructor(bigint?: boolean);
Expand Down Expand Up @@ -218,6 +220,8 @@ declare namespace InternalFSBinding {
function writeString(fd: number, value: string, pos: unknown, encoding: unknown, req: FSReqCallback<number>): void;
function writeString(fd: number, value: string, pos: unknown, encoding: unknown, req: undefined, ctx: FSSyncContext): number;
function writeString(fd: number, value: string, pos: unknown, encoding: unknown, usePromises: typeof kUsePromises): Promise<number>;

function getFormatOfExtensionlessFile(url: string): ConstantsBinding['fs'];
}

export interface FsBinding {
Expand Down Expand Up @@ -269,4 +273,6 @@ export interface FsBinding {
writeBuffer: typeof InternalFSBinding.writeBuffer;
writeBuffers: typeof InternalFSBinding.writeBuffers;
writeString: typeof InternalFSBinding.writeString;

getFormatOfExtensionlessFile: typeof InternalFSBinding.getFormatOfExtensionlessFile;
}

0 comments on commit 1e762dd

Please sign in to comment.