Treat any en-* dialect as default

[bpasero]

Today we seem to look for a language pack when e.g. en-GB is declared. But the fast path is to let it through as our default locale since we have no language pack for it as far we are concerned.

vscode/src/vs/base/node/languagePacks.js

Lines 126 to 130 in 963bd98

	// Check if we have an English or English US locale. If so fall to default since that is our
	// English translation (we don't ship *.nls.en.json files)
	if (locale && (locale === 'en' || locale === 'en-us')) {
	return Promise.resolve({ locale: locale, availableLanguages: {} });
	}

[TylerLeonhardt]

We use to have an en-GB language pack. I took it down because the localization team didn't want to maintain it. However there are folks that still want it (@Tyriar being one):

microsoft/vscode-loc#443

I've thought about offering tooling for folks to make languages packs... en-GB would be a perfect candidate for this.

So at first glance, I don't like this idea.

[Tyriar]

What's the performance impact of this? Is it just a single request one time for en-* language packs that gets cached after that?

[bpasero]

Around 15-20ms on my Windows machine. We mainly seem to be using require calls, so the stack ends up in node.js. My understanding is that unless you have en as locale or you have the locale set in argv.json, you will always have this watrfall.

[Tyriar]

@TylerLeonhardt is there a way this could be cached so it's only done on a cold start or when some change is detected? I'd say let's handle en specially like @bpasero is suggesting if not, we can always change it if there are ever other language packs.

[dbaeumer]

The require call you are seeing is here:

vscode/src/main.js

Line 563 in 70c7237

const { getNLSConfiguration } = require('./vs/base/node/languagePacks');

which loads the languagePacks.js file which isn't huge. So I guess the time we are seeing is actually spent in getNLSConfiguration() which tries to find a language pack on disk in case the locale is not en or en-US which we can extend to en-*. So we can't avoid that code for locales !== 'en'.

If we write the locale into argv.json we still need to spend that time. We could simply start doing it a little earlier. @bpasero do you now the time we spent between having the content of the argv.json file and the app ready event?

[bpasero]

app.on("ready") is mystical to us and can be anything from milliseconds to seconds: #22087

We track it in Kusto and I think its typically in the 100ms range.

[dbaeumer]

OK. Good to know.

[TylerLeonhardt]

I'm fine with treating all en locales like en but if we do get to a point where we have nice tooling around crafting language packs, I will want to remove that optimization to enable en-GB and all the rest.

[bpasero]

Yeah that was my suggestion.

Nevertheless, if we can revisit how NLS is blocking startup in a waterfall or not, I am also happy.

[Tyriar]

Is there any reason vs/base/node/languagePacks isn't included in the bundle if it's relatively small?

[dbaeumer]

The code is executed in the main thread and not the renderer. That is why it is not bundled into the renderer. To my 'old' knowledge we don't have a main bundle (@bpasero is this still correct). If we do we should definitely bundle it there.

[bpasero]

Yeah, at that point its just commonJS, not AMD.

[dbaeumer]

Could we use esbuild to bundle main.js? Then we don't need to use AMD.

[bpasero]

We can for the languagePacks.js but it would not help for:

vscode/src/vs/base/node/languagePacks.js

Lines 73 to 82 in a5c0f6c

	function getLanguagePackConfigurations(userDataPath) {
	const configFile = path.join(userDataPath, 'languagepacks.json');
	try {
	return nodeRequire(configFile);
	} catch (err) {
	// Do nothing. If we can't read the file we have no
	// language pack config.
	}
	return undefined;
	}

[dbaeumer]

Agree, but we could easily replace this with a async read since the call to getLanguagePackConfigurations is already in a async function.

[bpasero]

That would be very cool! To prevent sync fs reads.

[bpasero]

Opened #159894 for that with a PR.