Slow lockfile gen

[finnhodgkin]

Description of the change

#1262
Lockfile generation on my large work project takes over 3 minutes.
Adding a bunch of memoisation and combining many individual sqlite queries into one gets this down to 1.5 seconds.

Checklist:

• Added the change to the "Unreleased" section of the changelog
• Added some example of the new feature to the README
• Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

[f-f]

@finnhodgkin I am interested in merging this, perf is something we care about a lot. You can go ahead with the cleanup and we can get this merged. But I can also review before that if that's something you'd like.

[finnhodgkin]

In my head I was going to clean this up straight after pushing but got distracted. I'll sort it tomorrow.

The thing I could do with a hand on is the sqlite query. I think it's safe to leave un-paramatised because it's called with Array PackageName which is sanitised, but coming from postgres it just feels wrong. Spreading parameters hit the upper limit of params and a single array param just doesn't seem to be possible. Currently:

export const getMetadatasImpl = (db, names) => {
  let query = 'SELECT * FROM package_metadata WHERE name IN (@names)';
  return db.prepare(query).all({ names: names.map((n) => `'${n}'`).join(',') });
};

Was possibly just calling it incorrectly, will test - WiseLibs/better-sqlite3#1109

Went with WiseLibs/better-sqlite3#283 (comment) in the end which seems to work.

[finnhodgkin]

@f-f should be good to review now as long as CI goes smoothly.

[f-f]

This is great now 👏

We should adjust a few comments and names, but it's otherwise good to go

[f-f]

Is there any point in keeping getMetadata as well now that we have getMetadatas?

As an aside, data is a latin word that's already plural, so adding the s suffix doesn't feel right. How about renaming to getMetadataForPackages?

[finnhodgkin]

The singular version is still used in Command.Repl, Command.Publish and Command.Registry so I just left it alone.

Thanks, will swap to suggested name, had the same thought re datas but wasn't sure about a good replacement.

[f-f]

The singular version is still used in Command.Repl, Command.Publish and Command.Registry so I just left it alone.

Sure, but we can now call the new implementation passing in a singleton array [packageName]? Having two implementations for the same thing guarantees that they will go out of sync in the future, so I'd rather remove the singular version now.

[finnhodgkin]

Oh sure. Like this?

[finnhodgkin]

I could replace the function entirely but it feels useful being able to look up a single package without Map.lookuping the results.

[f-f]

Yeah that makes sense, thanks!

[f-f]

@finnhodgkin could you merge with latest?

[f-f]

Thank you!