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

NETSCRIPT: Rework disableLog for efficiency #1589

Merged
merged 1 commit into from
Aug 17, 2024

Conversation

d0sboots
Copy link
Collaborator

TL;DR: I'm seeing almost a 3x reduction in script size and a 10x performance increase in script launch + disableLog("ALL"). Numbers will be less impressive when actual batching logic is added. Old one could only get to ~1m scripts, new can easily get to 3m.


The current implementation was naive; disableLog("ALL") was storing a key for every function, and iterating over a different object to do it (when iterating over objects is quite slow).

The common cases of Bitburner (and especially batching, where efficiency matters most) are either never disabling anything, or disabling "ALL". This optimizes for these two cases, at the expense of slightly more complicated code to deal with the less-common edge cases.

Tests

Added a new test to verify some of the potential new edge cases

Performance

benchmark_disableLog.js

/** @param {NS} ns */
export async function main(ns) {
    const perf = globalThis.performance;
    const startMem = perf.memory.usedJSHeapSize;
    ns.tprintf("Starting memory: %d", startMem);
    let speedMax = 0;
    const totalScripts = Number(ns.args[0]);
    let start = perf.now();
    for (let i = 0; i < totalScripts;) {
        const k = Math.min(i + 10000, totalScripts);
        for (; i < k; i++) {
            if (ns.run("lib/disableLog.js", {temporary: true}) <= 0) {
              throw new Error("Out of memory: " + i);
            }
        }
        await new Promise(setTimeout);
        const end = perf.now();
        const speed = 1e7 / (end - start);
        start = end;
        speedMax = speed > speedMax ? speed : speedMax;
        ns.tprintf("Iter %6d: %.3f/sec", i, speed);
    }
    ns.tprintf("Best: %.3f/sec", speedMax);
    const endMem = perf.memory.usedJSHeapSize;
    ns.tprintf("Ending memory: %d", endMem);
    ns.tprintf("Used ~%.1f bytes/script", (endMem - startMem) / totalScripts);
}

lib/disableLog.js

/** @param {NS} ns */
export function main(ns) {
  ns.disableLog("ALL");
  // This will never resolve, and doesn't require defining a closure.
  return new Promise(Boolean);
}

I ran these tests in my browser, against dev BB before/after the change. Before:

run benchmark_disableLog.js 1000000
...
Iter 970000: 5863.039/sec
Iter 980000: 5844.194/sec
Iter 990000: 2724.350/sec
Iter 1000000: 4868.549/sec
Best: 7647.599/sec
Ending memory: 3383249365
Used ~2892.2 bytes/script

After:

run benchmark_disableLog.js 2000000
...
Iter 1970000: 41580.042/sec
Iter 1980000: 43029.260/sec
Iter 1990000: 44642.857/sec
Iter 2000000: 66137.566/sec
Best: 82576.383/sec
Ending memory: 2643886332
Used ~1083.6 bytes/script

The current implementation was naive; disableLog("ALL") was storing a
key for every function, and iterating over a different object to do it
(when iterating over objects is quite slow).

The common cases of Bitburner (and especially batching, where efficiency
matters most) are either never disabling anything, or disabling "ALL".
This optimizes for these two cases, at the expense of slightly more
complicated code to deal with the less-common edge cases.
@d0sboots
Copy link
Collaborator Author

@yichizhng for review, since you were the one to discover the issue.

@yichizhng
Copy link
Contributor

LGTM

I was considering an optimization where you store a default + keys which vary from the default, so that disabling all logs except one is more efficient, but I don't think that use case is important enough to optimize for.

@d0sboots d0sboots merged commit 34db6e8 into bitburner-official:dev Aug 17, 2024
5 checks passed
@d0sboots d0sboots deleted the disablelog branch August 17, 2024 02:10
antoinedube pushed a commit to antoinedube/bitburner-source that referenced this pull request Aug 21, 2024
The current implementation was naive; disableLog("ALL") was storing a
key for every function, and iterating over a different object to do it
(when iterating over objects is quite slow).

The common cases of Bitburner (and especially batching, where efficiency
matters most) are either never disabling anything, or disabling "ALL".
This optimizes for these two cases, at the expense of slightly more
complicated code to deal with the less-common edge cases.
Faenre pushed a commit to Faenre/bitburner-src that referenced this pull request Aug 22, 2024
The current implementation was naive; disableLog("ALL") was storing a
key for every function, and iterating over a different object to do it
(when iterating over objects is quite slow).

The common cases of Bitburner (and especially batching, where efficiency
matters most) are either never disabling anything, or disabling "ALL".
This optimizes for these two cases, at the expense of slightly more
complicated code to deal with the less-common edge cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants