Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement native os module #1115

Merged
merged 118 commits into from
Aug 26, 2022

Conversation

xhyrom
Copy link
Collaborator

@xhyrom xhyrom commented Aug 20, 2022

Warning
In huge WIP

Note
Implemented, our first PR (@Fire-The-Fox, @xhyrom)

  • tests
  • bun-types
  • faster than other runtimes
  • benchmarks
    • cpus()
    • networkInterfaces()
    • arch()
    • endianness()
    • freemem()
    • getPriority()
    • homedir()
    • hostname()
    • loadavg()
    • platform()
    • release()
    • setPriority()
    • tmpdir()
    • totalmem()
    • type()
    • uptime()
    • userInfo()
    • version()

Required before ready to review:

  • Install MacOS on VMWare (in progress, my net is slow)
  • Make support for MacOS
    • cpus()
    • freemem()
    • loadavg()
    • networkInterfaces()
    • uptime()
    • version()
    • release()
  • Test every function
    • linux
    • darwin
import os from "node:os";

for (const p of Object.getOwnPropertyNames(os)) {
    if (p === "setPriority" || p === "default") continue;

    if (typeof os[p] === 'function') console.log(p, os[p]());
    else console.log(p, os[p]);
}

console.log(`priority ${os.getPriority()} (should be 0)`);
os.setPriority(0, 2);
console.log(`priority ${os.getPriority()} (should be 2)`);
os.setPriority(4);
console.log(`priority ${os.getPriority()} (should be 4)`);

Must be implemented:

  • os.EOL
  • os.arch()
  • os.cpus()
  • os.devNull
  • os.endianness()
  • os.freemem()
  • os.getPriority() - missing info field in error
  • os.homedir()
  • os.hostname()
  • os.loadavg()
  • os.networkInterfaces()
  • os.platform()
  • os.release()
  • os.setPriority() - missing info field in error
  • os.tmpdir()
  • os.totalmem()
  • os.type()
  • os.uptime()
  • os.userInfo() - missing encoding option
  • os.version()
  • os.constants
    • os.constants.signals
    • os.constants.errno
    • os.constants.dlopen
    • os.constants.priority
    • UV_UDP_REUSEADDR
// RUN WITH NODEJS NO BUN
const os = require("node:os");

let declarations = "";
let addToModule = "";
for (const [key, value] of Object.entries(os.constants.signals)) {
    declarations += `   pub const ${key} = ${value};\n`;
    addToModule += `    constantsModule.put(globalObject, &JSC.ZigString.init("${key}"), JSC.JSValue.jsNumber(${key}));\n`
}

console.log([
    declarations,
    '',
    'pub fn create(module: JSC.JSValue, globalObject: *JSC.JSGlobalObject) callconv(.C) void {',
    `   const constantsModule = JSC.JSValue.createEmptyObject(globalObject, ${Object.keys(os.constants.signals).length});`,
    `   ${addToModule}`,
    '   module.put(globalObject, &JSC.ZigString.init("dlopen"), constantsModule);',
    '}',
].join('\n'));

Big ❤️ for @Fire-The-Fox . He help me with cpus and networkInterfaces

@xhyrom xhyrom marked this pull request as draft August 20, 2022 08:47
@xhyrom xhyrom changed the title feat: implement native os module [WIP] feat: implement native os module Aug 20, 2022
@xhyrom xhyrom linked an issue Aug 20, 2022 that may be closed by this pull request
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this

Sorry that the patterns in to copy from in the code are unclear. The newer way of writing bindings like this -- using JSC.NewFunction() should be simpler and a little faster at runtime

@@ -486,6 +486,18 @@ pub fn getImportedStyles(
return JSValue.createStringArray(ctx.ptr(), styles.ptr, styles.len, true).asRef();
}

pub fn newOs(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a new and better way to expose ESM modules, but most of the code hasn't been migrated to it. Have a look at ProcessModule.h.

Instead of adding a new property and writing JS wrapper code, we can directly expose the JSValue objects

@@ -0,0 +1,25 @@
function bound(obj) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete this file if we use the same way as ProcessModule.h

@github-actions github-actions bot added the needs tests Something that needs more testing label Aug 20, 2022
@xhyrom xhyrom requested a review from Jarred-Sumner August 20, 2022 21:02
@xhyrom
Copy link
Collaborator Author

xhyrom commented Aug 25, 2022

not finished corrupted double-linked list

@xhyrom
Copy link
Collaborator Author

xhyrom commented Aug 25, 2022

@Jarred-Sumner everything works, tested:

import os from "node:os";

for (const p of Object.getOwnPropertyNames(os)) {
    if (p === "setPriority" || p === "default") continue;

    if (typeof os[p] === 'function') console.log(p, os[p]());
    else console.log(p, os[p]);
}

console.log(`priority ${os.getPriority()} (should be 0)`);
os.setPriority(0, 2);
console.log(`priority ${os.getPriority()} (should be 2)`);
os.setPriority(4);
console.log(`priority ${os.getPriority()} (should be 4)`);

console.log('STARTING 100 TIMES - CP');
for (let i = 0; i < 100; i++) {
    os.cpus();
}

console.log('STARTING 100 TIMES - NT');
for (let i = 0; i < 100; i++) {
    os.networkInterfaces();
}

but you should test it on darwin :D - we cant, because VM

@Jarred-Sumner Jarred-Sumner merged commit 7a734e0 into oven-sh:main Aug 26, 2022
@Jarred-Sumner
Copy link
Collaborator

awesome work @xhyrom @Fire-The-Fox

@xhyrom xhyrom mentioned this pull request Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Something that needs more testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

os.arch() and os.platform() are not correct
3 participants