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

Dex/loadData: Merge objects w/ spread syntax #10639

Merged

Conversation

larry-the-table-guy
Copy link
Contributor

@larry-the-table-guy larry-the-table-guy commented Oct 28, 2024

Results in fewer object shapes.

Similar to #10608. Smaller effect, but very easy change.

loadData and construct all are in milliseconds.
heap 1 is MB after loading data for all mods (after)
heap 2 is MB after constructing objects for all mods (aftest)

Stage loadData construct all heap 1 heap 2
Baseline 1261 3421 174 277
just loadData (this PR) 1265 3052 171 268
just field order (other PR) 1274 2197 174 263
both PRs 1264 2064 171 260

Benchmark code

Inserted at bottom of ./sim/dex.ts

const v8 = require('v8');
v8.writeHeapSnapshot(); // before
const start1 = performance.now();
Dex.includeMods();
Dex.includeModData();
const loaded_data_t = performance.now();
v8.writeHeapSnapshot(); // after
const start2 = performance.now();
for (const d in dexes) {
        if (d === 'base') continue;
        const m = dexes[d];
        m.species.all();
        m.moves.all();
        m.items.all();
        m.abilities.all();
        m.types.all();
        m.natures.all();
}
const constructed_t = performance.now();
v8.writeHeapSnapshot(); // aftest

console.log('loadDatas', loaded_data_t - start1);
console.log('constructors', constructed_t - start2);
throw new Error('stop early');

Results in fewer object shapes
@Zarel Zarel merged commit 13ee244 into smogon:master Oct 29, 2024
1 check passed
@Zarel
Copy link
Member

Zarel commented Oct 29, 2024

Thanks!

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