-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Initializing the "ja" culture takes 200ms when using ICU #31273
Comments
@adamsitnik does the results change if we change the initialization yield return new CultureInfo("ja"); to yield return CultureInfo.GetCultureInfo("ja"); Sorry I didn't try it myself yet. I am asking because the parsing code should be exact the same between Windows and Linux, the only difference is the calendar data we pick from the OS which should be cached if you are not recreating the culture. |
I've just tried and it does not change. I've also tested |
@adamsitnik thanks for your quick try. |
I looked at that and I am seeing the problem is the parse initialization time is counted as part of the measurements. The initialization should occur once for the culture. The test should really measure the parsing perf excluding the one time parse initialization. when I change the test code to: private static CultureInfo s_jaCulture = CultureInfo.GetCultureInfo("ja");
private static CultureInfo s_jaJPCulture = CultureInfo.GetCultureInfo("ja-JP");
private CultureInfo JA { get { DateTime.Parse("10/10/2010 12:00:00 AM", s_jaCulture); return s_jaCulture; }} // call parsing one time to force initialization
private CultureInfo JAJP { get { DateTime.Parse("10/10/2010 12:00:00 AM", s_jaJPCulture); return s_jaJPCulture; }} // call parsing one time to force initialization
public IEnumerable<object> Cultures()
{
// yield return new CultureInfo("ja");
yield return JA;
yield return JAJP;
yield return new CultureInfo("fr");
yield return new CultureInfo("da");
yield return new CultureInfo("");
} I am getting the results:
It is expected the Japanese culture parsing initialization be slower than other cultures because Japanese cultures have multiple eras which required to initialize more stuff. @adamsitnik let me know if you want me to update the test as I did here. |
I've investigated this issue and it turned out to be more complex ;) BenchmarkDotNet JITs the code by just running the benchmark once. When it takes a lot of time, it skips further warmups and just runs the benchmark once per iteration. It does so to avoid situations where we have a benchmark that takes a lot of time to run and we run it many times just for a warmup (for example for 30s per run and six warmups = 3 minutes spend for a warmup). The problem are benchmarks where the initialization time is high, but every next run is short. dotnet/BenchmarkDotNet#837 In this case, the first call to following code: var culture = new CultureInfo("ja");
var result = DateTime.Parse("10/10/2010 12:00:00 AM", culture); takes 200 ms to execute and this is not the cost of JITting the code (I've run it with CoreRT for comparison). With the performance repo settings ( The initialization time can be easily measured using the using System;
using System.Globalization;
namespace culture
{
class Program
{
static void Main(string[] args)
{
var culture = new CultureInfo("ja");
var result = DateTime.Parse("10/10/2010 12:00:00 AM", culture);
}
}
} time dotnet run -c Release --no-build --no-restore
While for a Hello World it's just:
I am going to rename this issue and investigate the initialization time. 200ms is way too long. |
I've studied the ICU UCollator APIs and I cant' see any alternatives to Currently the only idea I have is to try to call this method with normalization disabled ( |
@adamsitnik would it help to have the ability to specify the number of warmup iterations as a manual override? |
@adamsitnik I am not sure it is worth it to try optimizing the initialization time of the parsing. The reason is the main scenarios always using a single culture (e.g. CurrentCulture or InvariantCulture) which have the initialization done once and we never got any complaint about that. Also, any code using CultureInfo.GetCultureInfo will always use cached culture with the cached parsing data. I suggest closing this one until we get a real complaint. |
Thanks @adamsitnik. I don't think this should be tagged for 5.0 though if you agree this is not a major issue we should target now. moving to 6.0 would be reasonable I guess. |
I agree, moving to 6.0 |
Could the issue title be updated so that this issue be more discoverable? For example, "when using ICU" instead of "on Linux". This issue affects also Windows and macOS now that .NET uses ICU on Windows by default and supports macOS. Regarding the issue itself... This issue has visible impact on desktop apps. For desktop apps, startup time is an important factor, especially for short-lived ones. Since .NET Core 3.1 is reaching its EOL, I expect more Windows desktop apps will encounter this issue. (On the other hand, server apps should be less affected because I believe they tend to use "en" or the invariant culture after all.) My actual apps, which are short-lived build tools utilizing Microsoft.Build.Construction APIs, encountered this issue after migrating from .NET Core 3.1 to .NET 6. Its execution time increased from ~1s to ~1.3s on average. As build tools they were executed thousands of times during a build, so the increase added up to hundreds of seconds. (Fortunately Globalization Invariant Mode was acceptable.)
Configuration:
|
Yes, I have updated the title. Thanks for the suggestion. |
If look PowerShell startup scenario (гsers often complain that it starts up slowly) it would be great to have fast ICU initialization. |
DateTime.Parse
for "ja" culture (ONLY) is 10x times slower on Linux compared to WindowsThe contributor who wants to work on this issue should:
The text was updated successfully, but these errors were encountered: