Optimizations & Chinese i18n

[CIVITAS-John]

The optimizations might cause a bit more redundant code, but they are very frequently executed, so I believe they are worthwhile.

Note that I removed a duplicate key-value pair when translating the English error messages to Chinese.

There is also a TU-specific error message but in the future, the features might be brought back to NLW as an extension.

We probably need to think about a way for extension i18n when we open up NLW extensions to the public.

[CIVITAS-John]

I am not sure why the check failed. Seems to point to a memory issue?

[CIVITAS-John]

Generally, I think the performance boost of gte and lte could be significant but probably not patchAtHeading stuff. If you look at how the gte was implemented before, the performance would actually depend on how likely the check is going to fail -

For example, in gte,

• When a > b, only gt is performed;
• When a <= b, gt & eq are performed.

Considering the many branches in eq, and many irrelevant branches, the boost could be considerable.

Alas, we do need to have a comprehensive benchmark that runs on a set of models... If time permits, I will find a VRF intern to work on that.

[CIVITAS-John]

Just added comments as requested. Thanks!

[CIVITAS-John]

To answer your question

What is the purpose for adding messageKey to the RuntimeException? I don't see that used anywhere. If it's not really doing anything, I'd prefer to remove it.

The purpose is to let TU know the exception message BEFORE localization - so that we can add other help information for learners to debug the issue. Without the message key, it is harder to decipher since errors are multilingual and with template parameters now.

[LaCuneta]

The purpose is to let TU know the exception message BEFORE localization - so that we can add other help information for learners to debug the issue. Without the message key, it is harder to decipher since errors are multilingual and with template parameters now.

Gotcha, that makes sense.

Now that I understand that I looked back at it and I think something is missing to complete that change. The @messageKey is added to the RuntimeException constructor and the calls of exceptions.runtime() in the validator, but not to the exceptions.runtime() arguments list or to the actual call of the constructor in that method. I think once that is added in this should be ready to merge.

[LaCuneta]

Hey John, the last couple commits had messages that could be more helpful. We'd prefer to keep our commit messages meaningful on their own, so "Jeremy's request" wouldn't mean much without the context of this PR. The "Merge branch..." one is similarly not descriptive of the actual changes there. You don't need to amend those commits and force push, but just letting you know for future reference.