Community
Participate
Working Groups
We should provide a flexible translation service inside e4
Created attachment 183968 [details] patch
Created attachment 184155 [details] patch * Fully implement * JUnit-test to check the API Todo: Wait for Equinox to finish the lookup completely so that files without an ending are interpreted as en or a custom value. Oleg? Can you review my work?
Created attachment 185001 [details] patch * Translation Service Implementation * PropertiesTranslationProvider (copied logic from Equinox ManifestLocalization) adjusting to bring the base bundle into play before faling back to the default locale in case Locale.getDefault() equals equinox.root.locale * JUnit Tests (no all passing the spec found in ResourceBundle#getBundle())
Oleg, can you take a look. I'd like to release this ASAP so that I can go on with the other areas we need for translations in XMI and e4 applications, thanks
(In reply to comment #0) > We should provide a flexible translation service inside e4 This is the part that needs to be more detailed. For 3.x we have NLS and extension registry mechanism. What are the shortcomings of the existing approaches that this enhancement solves? How do you expect developers to migrate to this new thing - what's the carrot? What are the downsides? Few more specific things, if you go with this approach: - translation providers: I'd rather see them as OSGi services with filters. Then translation service does not have to worry about registering / unregistering them. - categories - how do I know what category to use? - context integration - I'd hope that translation picks up desired locale information from the context - performance, caching, especially startup performance - PropertiesBundleTranslationProvider does not look like it needs to be an API. - throwing exceptions from #translate() methods - exception catching is expensive both in CPU and lines of code; I'd rather have it return the original key if it can't translate - Locale class - we had issues with round-tripping between Locale and its String representation. It is safer to pass around the string form ("EN_ca") === The most important point is at the beginning - what are the advantages that would make developers use this instead of NLS and what is the migration path?
On what are the advantages: * It allows dynamic language switching * It allows multiple locales at once * It is not bound to .properties-Files On OSGi services with filters: * I'm not sure how you imagine this to work so maybe you can show me some pseudo code. Does this mean instead of remembering those thing internally in the map i look them up in service registry On category: * I'm not sure what you are referring here - the developer knows about them because he/she registers the provider for it. Categories always available are the bundle names On context: * I'm using not sure what you mean about this, if no Locale is provided I'm using the LocaleProvider which is context aware. The translation context is something where we need a service at the workbench level which is the context which is provided through DI (e.g. LanguageService). What we have here is the foundation which provides a locale string for a given Locale context is one level above this service - maybe we should remove the methods who don't take a Locale On performance: * As outlined in the original bug I think we are in Eclipse 4.0 SDK in a much better position than we are in 3.x because we can do the translations gradually and not at start up because they are decorations in the renderer On PropertiesBundleTranslationProvider: * It is a base thing to contribute your own filebase translation provider On exception throwing: * Ok taken I'll fix that On Locale vs String: * Ok taken no problem On Transition paths for NLS: 3.x: ---- Java-Class: package mypackage; public class MyMessage { public static String myKey; static { NLS.initialize(MyMessages.class); } } Properties-File: mypackage/mymessage.properties: myKey = My Message mypackage/mymessage_de.properties: myKey = Meine Nachricht Usage: public class MyView extends ViewPart { public void createPartControl(Composite parent) { Label l = ... l.setText(MyMessage.myKey); } } 4.x: ---- Java-Class: public class MyMessage { public String myKey; } Properties-File: same as 3.x public class MyView { @Inject public void postInit(Composite parent, @Translation MyMessage messages) { Label l = ... l.setText(messages.myKey); } } Additional things: * reinjected @Translation when language changes * default look up is MyMessage => mymessage.properties which can be modified by annotating MyMessage like this: @Messages(providerid="mydblookup") public class MyMessage { public String myKey; } now the lookup is not by searching for the mymessage.properties (and registering a provider with this ID) but using the here defined service with the given providerid
Oleg - I think it is easier to work on this when we can share it in CVS. I've created a branch translation_branch in CVS and will as a next step fix the API to not use Locale
I've fixed the following things: * API is only useing Strings now no Locale anymore * The API methods who don't require a Locale are removed we are doing translations so there's no magic needed
(In reply to comment #8) > I've fixed the following things: > * API is only useing Strings now no Locale anymore so the strings we pass around (presumably from the IEclipseContext) will be able to handle the Locale extensions? Example of the old format from http://www.unicode.org/reports/tr35/ : eg@collation=phonebook;calendar=islamic-civil;currency=USD PW
(In reply to comment #9) > (In reply to comment #8) > > I've fixed the following things: > > * API is only useing Strings now no Locale anymore > > so the strings we pass around (presumably from the IEclipseContext) will be > able to handle the Locale extensions? > > Example of the old format from http://www.unicode.org/reports/tr35/ : > > eg@collation=phonebook;calendar=islamic-civil;currency=USD > > PW no - all we understand is what you get from Locale#toString() but I'm curious Oleg what's been the problem with Locale?
(In reply to comment #10) > no - all we understand is what you get from Locale#toString() but I'm curious > Oleg what's been the problem with Locale? Sorry, I'm saying this is a requirement. We need to be able to handle this in any code that supports the Eclipse SDK (and that means the Modeled Workbench). PW
Can you please tell me more about this I really don't understand what this has to do with the translation service.
(In reply to comment #12) > Can you please tell me more about this I really don't understand what this has > to do with the translation service. In a running SDK the Locale can sometimes provide this information (the full string) instead of just "eg" for Egyptian. For translations we just have to make sure that we get the same translation for "eg" as for "eg@collation=phonebook;calendar=islamic-civil;currency=USD" I would suspect we might need to pass around the entire string if it can effect BiDi, collation, or if we need to re-create the Locale objects at some point (I'm still don't have a specific usecase, but we just need to be aware while working on this). PW
(In reply to comment #8) > * The API methods who don't require a Locale are removed we are doing > translations so there's no magic needed I was hoping for the opposite. The locale is to be taken from the context and normally developer won't have to worry about specifying it.
(In reply to comment #6) > On OSGi services with filters: > * I'm not sure how you imagine this to work so maybe you can show me some > pseudo > code. You could register translation providers as OSGi services for the same class (ITranslationProvider, for instance) and a property specifying the category, something like {"TRANSLATION_CATEGORY", "abc" }. The use a filter to get that translation provider. See BundleContext#getServiceReferences(String clazz, String filter) and #registerService(String[] clazzes, Object service, Dictionary<String, ? > properties) for a starting point and OSGi core spec 5.2.5 "Service Properties, 5.5 "Filters" for more details. To see why you'd want to avoid adding an extra layer in registering / unregistering services, consider what should be TranslationService's reaction to bundles being added/removed. > On category: > * I'm not sure what you are referring here - the developer knows about them > because he/she registers the provider for it. Categories always available are > the bundle names In the NLS world the analog of the "category" is in one place - the NLS file itself. I'd venture to guess that most developer don't think about it once it has been setup. You proposing that every translation call to include category which in almost all cases will be the name of the bundle containing the code? > > On context: > * I'm using not sure what you mean about this, if no Locale is provided I'm > using the LocaleProvider which is context aware. The translation context is > something where we need a service at the workbench level which is the context > which is provided through DI (e.g. LanguageService). What we have here is the > foundation which provides a locale string for a given Locale context is one > level above this service - maybe we should remove the methods who don't take > a > Locale I'd hope to have locale information (which by the way needs to be derived from both environment variables and runtime options) placed in the application context. (With the understanding that child contexts can then override it.) > > On performance: You saying that performance of translation does not matter because only very little of it will be done at any given time. While I haven't run tests, from the top of my head this does not seem to be correct. Consider, for instance, that I right-clicked on a file in Package Explorer. There will be a few bundles contributing to that; say, 10 bundles for a simple product. Say, my language is set to "FR_ca". For each of those 10 bundles we'll need to check if they have a file "properties_FR_ca", then, possibly, "properties_FR", then, possibly "properties". Then we need to open such file, parse, and find an entry for the specific command's lable in it. At this point we have up to 30 hard drive access requests done, and this is for a very basic scenario. Compare this to the 3.x story where translation is done *one* time in the typical application life and is repeated only when environment or bundle set changes. In 3.x the CPU translation cost is pretty much zero, and that is what developers will expect from 4.x.
(In reply to comment #14) > (In reply to comment #8) > > * The API methods who don't require a Locale are removed we are doing > > translations so there's no magic needed > > I was hoping for the opposite. The locale is to be taken from the context and > normally developer won't have to worry about specifying it. I think we have a real misunderstanding what this ITranslationService is for. It is not meant to be consumed by externals but only by framework components and the revised "MessageKey-Instance-I'm not a static implementation". Think of it as a generic translation service and normally if you'd like to translate a value you know which language you want it to. The context is logically a level above at the workbench.
[...] > In the NLS world the analog of the "category" is in one place - the NLS file This service is NOT meant to be a replacement for NLS the replacement for NLS is: > itself. I'd venture to guess that most developer don't think about it once it public class MyMessage { public String myKey; } Properties-File: same as 3.x public class MyView { @Inject public void postInit(Composite parent, @Translation MyMessage messages) { Label l = ... l.setText(messages.myKey); } } > has been setup. You proposing that every translation call to include category > which in almost all cases will be the name of the bundle containing the code? > As outlined in the other response I don't think this service will be directly used by developers. This service is used by: * RenderingEngine * @Translation-DI engine
[...] > > > > On performance: > > You saying that performance of translation does not matter because only very > little of it will be done at any given time. While I haven't run tests, from > the top of my head this does not seem to be correct. > As outlined in my mail to e4-dev the performance, memory usage, ... heavily depends on the ITranslationProvider implementation. This first implementation uses what is provided to us by OSGi. I'll try to write one which work exactly the same than the one in extension registry, then the only overhead is a method call to 3.x. The then question is if it wouldn't be better to make the BundleLocalization-Service provide such a performance feature because else e4 and extension-registry do the same.
(In reply to comment #15) > (In reply to comment #6) > > On OSGi services with filters: > > * I'm not sure how you imagine this to work so maybe you can show me some > > pseudo > > code. > > You could register translation providers as OSGi services for the same class > (ITranslationProvider, for instance) and a property specifying the category, > something like {"TRANSLATION_CATEGORY", "abc" }. The use a filter to get that > translation provider. > > See BundleContext#getServiceReferences(String clazz, String filter) and > #registerService(String[] clazzes, Object service, Dictionary<String, ? > > properties) for a starting point and OSGi core spec 5.2.5 "Service Properties, > 5.5 "Filters" for more details. > > To see why you'd want to avoid adding an extra layer in registering / > unregistering services, consider what should be TranslationService's reaction > to bundles being added/removed. > DS handles this pretty well
Created attachment 185457 [details] Performance comparison So I was curious how big the overhead of translation loading is I've tested with 20 bundles a 100 keys and 3 bundle-files (bundle, bundle_fr, bundle_fr_FR): Cold start & fallback to default (es_ES): Extension: 50 Extension Second: 2 Service: 13 Service Second: 10 Cold start & fallback to language (fr): Extension: 53 Extension Second: 1 Service: 11 Service Second: 2 Cold start & perfect match (fr_FR): Extension: 58 Extension Second: 1 Service: 11 Service Second: 2 Warm start & fallback to default (es_ES): Extension: 29 Extension Second: 2 Service: 46 Service Second: 4 Warm start & fallback to language (fr): Extension: 31 Extension Second: 1 Service: 42 Service Second: 2 Cold start & perfect match (fr_FR): Extension: 31 Extension Second: 1 Service: 47 Service Second: 2 The numbers are milliseconds, something I really don't understand is why the hell the service stuff is so slower on a warm start. I'd expect extension registry getting faster on warm start and the service stuff equally fast on both. Is there something wrong with my test, can anyone explain this behavior?
(In reply to comment #20) > The numbers are milliseconds, something I really don't understand is why the > hell the service stuff is so slower on a warm start. I'd expect extension > registry getting faster on warm start and the service stuff equally fast on > both. Is there something wrong with my test, can anyone explain this behavior? - You use System.currentTimeMillis(). While it says "ms" it has granularity of about 13 - 15 ms on Windows machines. So, your results are +/30 ms. Better: -> use System.nanoTime() - in my experience it is still accurate to only about ~2ms, but that's better then 15ms; -> do a series of runs in a loop, discarding the first run as it will be heavily influenced by class loading (unless you want to include clasloading, which requires different test setup) -> keep in mind that a small number of runs is unlikely to produce interesting results, the rule of thumb is that measured Java code has to take > 100 ms, while large number of will force JIT optimization to kick in more progressively -> basic statistical analysis often needed to make sense of results, especially of you work with times < 20ms. - On the extension registry, it loads caches in blocks, so the first access to the block is more expensive then the rest, even in a "warm" state
(In reply to comment #21) > (In reply to comment #20) > > The numbers are milliseconds, something I really don't understand is why the > > hell the service stuff is so slower on a warm start. I'd expect extension > > registry getting faster on warm start and the service stuff equally fast on > > both. Is there something wrong with my test, can anyone explain this behavior? > > - You use System.currentTimeMillis(). While it says "ms" it has granularity of > about 13 - 15 ms on Windows machines. So, your results are +/30 ms. > > Better: > -> use System.nanoTime() - in my experience it is still accurate to only about > ~2ms, but that's better then 15ms; Ok. Cold start fr_FR: ----------------- Extension: 71 Extension Second: 2 Service: 11 Service Second: 2 Warm start fr_FR: ----------------- Extension: 28 Extension Second: 2 Service: 50-65 Service Second: 2 > -> do a series of runs in a loop, discarding the first run as it will be > heavily influenced by class loading (unless you want to include clasloading, > which requires different test setup) This is not really important in our scenario because only first time access is important. As you can see from the second values this is equally fast for both. > -> keep in mind that a small number of runs is unlikely to produce interesting > results, the rule of thumb is that measured Java code has to take > 100 ms, > while large number of will force JIT optimization to kick in more progressively I tested your requested Menu scenario 20 bundles contributing to the popup menu and I think the time spend in translation 28 vs 50 ms is acceptable. > -> basic statistical analysis often needed to make sense of results, > especially of you work with times < 20ms. well I've launched now about 20-30 times and the numbers are all the time around this with some very few exceptions. > > - On the extension registry, it loads caches in blocks, so the first access to > the block is more expensive then the rest, even in a "warm" state IMHO second access are not important even if extension registry would be 100 times faster it would not make any difference to the user. At least to me these figures say that at least the performance from the currently completely unoptimized translation-service is sufficient because if we assume no bundle has been loaded before a menu is shown where 20 bundles contribute to we talk about 50ms and i highly doubt the user notices that.
what i still dont't understand is the performance regression for the service stuff in warm starts.
So here's my current state: - There's a branch named "translation_branch" for the following bundles: * core.services * ui.services * ui.workbench * demo.contacts - There are 2 OSGi-Services * ITranslationService: Generic Translation Infrastructure * IMessageFactoryService: Replacement for NLS stuff * @Message: To configure message loading - DI-Service and Injection * ETranslationService: Language Context Aware Service * @Translation: To get Message-Instances injected I've not modified by current setup to do the look up of ITranslationProvider through OSGi because I can't see any major advantage to DS I'm using currently. I think this current implementation clears up some of Oleg raised in his comment 15. A problem I'm having with TranslationObjectSupplier is to get access to the current injection context to find the language aware ETranslationService (I had to get access to internal-classes)
Oleg - I think we need to find a conclusion. If you have time ping me on IRC
Yes, I am looking into this. The real requirement here is that we translate string in e4xmi files with the possible bonus of being able to dynamically switch the language. Somehow it feel that the recent patches gone off in a different direction (which is an interesting one, but not the immediate issue we have in 4.x code).
The only interesting things for e4xmi translation is: * ITranslationService & ITranslationProvider: which provides a key based lookup * ETranslationService: which holds the language context if you are fine with those two everything else is an addon we could also provide outside of the core framework because it builds upon this simple API.
(In reply to comment #27) > The only interesting things for e4xmi translation is: > * ITranslationService & ITranslationProvider: which provides a key based lookup > * ETranslationService: which holds the language context > > if you are fine with those two everything else is an addon we could also > provide outside of the core framework because it builds upon this simple API. Yes, that's a good start, let's concentrate on those APIs first. ===> ITranslationService <=== translate(String locale, String providerId, String key) public String[] translate(String locale, String providerId, String... key); There will be one "standard" translation service for the model. So, #translate() methods need to be take in the contributorURI in order to find "default" properties file. The service will be obtained from a context, either the model element's context or a parent model element with a context. The context will have current locale (the field that needs to be declared as API and recieve default value on startup). As specific implementation of translation service is obtained from the context, there is no need to specify providerID. So, methods probably need to be more like this: translate(String key, String contributorURI) translate(String... key, String contributorURI); and at this time I'd rather only provide the method with one "key" argument as most UI calls we have only work on a single argument. So, the ITranslationService becomes one method String translate(String key, String contributorURI) While there I'd remove most of the current Javadoc on this class. It is both confusing and based too much on the underlying implementation using properties-like resources. I'd just state the purpose and describe the context key from which locale is taken. ===> ITranslationProvider <=== public String translate(String locale, String key); I'd expect that the implementation of this interface is to be injected with a context and that it would take locale information from the context. Also, as one provider will serve multiple bundles it needs to take contributorURI as an argument. The objects that we expect customers to implemented are better represented as abstract classes (it leaves us more space for future adjustments) so it will become something like: abstract class AbstractTranslationProvider { @Inject @Named("E4.Locale") protected String locale; @Inject public AbstractTranslationProvider() { // placeholder } abstract public String translate(String key, String contributorURI); } ===> ETranslationService <=== This is not really needed for e4xmi; I'd leave it for future. (The only potential benefit I can see if the propagation of changes, but that will happen only to consumer's classes. They would have to add code to propagate updated values to SWT or whatever renders UI and that will completely negate the advantage: it will be easier to listen to locale change in the context and trigger update on that.) Instead what could be useful is a "databinding" mechanism where instead of Button#setText() consumer's code would say "BindText(key, button)" and "BindTooltip(key, button").
Ok. I think it's hard for me to understand where you are going without seeing your code in action. What is really important for me is that translations have to come not only from .properties-File.
Created attachment 186664 [details] Patch alternative This is a patch against CVS Head. In this approach: - context has the relevant locale (TranslationProvider.LOCALE) - context has an implementation of TranslationProvider service - the default locale is taken from default locale ("-nl", "osgi.nl") - the default translation service is bundle-based and the same as used for plugin.xml Both locale and translation service implementation can be overridden in the context (either the top level context or a specific context).
Created attachment 186665 [details] Patch alternative demo This is a demo on how the service can be used. The "File" menu in the contacts demo is translated; set runtime "-nl" option to "-nl ru" to see it shown in Russian. This patch is an illustration only and is not meant to be applied; I'd like to see if we can confine translation to the internals of the model.
Oleg - this looks fine! So +1 from me to get this code into CVS. How do you see my proposal of loading translations into class instances instead of the static approach followed currently. I ask because when looking at our e4 code base it is using the static style stuff which means we are unable to switch the language at runtime.
Probably we could we enhance the lookup so that one can pass a version range e.g. platform:/plugin/my.bundle?version=[1.0.0,2.0.0) in future?
One more question Oleg. How can one replace the TranslationProvider for model keys? Say I'd like the en translations coming from the BundleLocalization but all others e.g. by looking them up in a database? This means I have to replace the TranslationProvider registered in the application IEclipseContext, right? Should we provide people a helper method to do such a thing else they'll have to find out the application IEclipseContext to inject their own provider.
Created attachment 186735 [details] Patch alternative demo - replacing translation service (In reply to comment #34) > One more question Oleg. How can one replace the TranslationProvider for model > keys? I too wanted to make sure it worked so I made this demo (patch against CVS Head, not meant to be applied as is). It uses model life cycle hook to replace translation service to provide a hexadecimal representation of the key itself. For the hook to be picked up, patched contacts demo has to be run with the argument: -lifeCycleURI platform:/plugin/org.eclipse.e4.demo.contacts/org.eclipse.e4.demo.contacts.model.internal.ModelLifeCycleExtension (obviously, all on one string). I updated the base class a bit so I am attaching the whole patch here, but really the points of interest are - ModelLifeCycleExtension#overrideTranslation(), and - BinaryTranslatorProvider class. The intention is that translation service can be overridden on a part of the model; for instance, by overriding the value in the perspective's context. > Should we provide people a helper method to do such a thing ... ? I'd rather provide a snippet (wiki? help page?) with an example of how it can be done. Very few people will be doing this, and helper method might be restrictive comparing to manipulating contexts.
(In reply to comment #35) > The intention is that translation service can be overridden on a part of the > model; for instance, by overriding the value in the perspective's context. Should be: "translation service can *also* be overridden for a part of the model"
(In reply to comment #33) > Probably we could we enhance the lookup so that one can pass a version range > e.g. platform:/plugin/my.bundle?version=[1.0.0,2.0.0) in future? Yes, eventually we'll need to either enhance the "platform" schema in Equinox or create a new schema.
(In reply to comment #32) > Oleg - this looks fine! So +1 from me to get this code into CVS. How do you see > my proposal of loading translations into class instances instead of the static > approach followed currently. > > I ask because when looking at our e4 code base it is using the static style > stuff which means we are unable to switch the language at runtime. That was the can of worms I tried to avoid so far :-). Replacing NLS is [much] more involved than what's in the patches. - Tooling: NLS warnings, templates - Performance: great deal of effort went into making it perform well - Buy-in value: if there is a new way of NLSing, it should provide a clear incentive to switch to a "typical" developer Is this an interesting direction? Yes, very much so. But existing NLS establishes a barrier of entry for new functionality.
Created attachment 186742 [details] Patch alternative v.2 Updated translation service patch. Patch applied to CVS Head.
Created attachment 186743 [details] Patch alternative v.2 demo This is the updated illustration of using translation services using contacts demo. This patch is not meant to be applied. To see the menu "File" translated into Russian, launch patched contacts demo with "-nl ru". To see the menu "File" translated into hexadecimal, launch patched contacts demo with "-lifeCycleURI platform:/plugin/org.eclipse.e4.demo.contacts/org.eclipse.e4.demo.contacts.model.internal.ModelLifeCycleExtension"
Created attachment 186844 [details] Patch alternative v.2 - model update & usage => Changes to UILabel: - removes fields localLabel, localTooltip, localImage - adds methods getLocalizedLabel(), getLocalizedTooltip() (Note that I did not add Image localization; there is FileLocator#find(Bundle bundle, IPath path, Map override) that provides that.) => Added LocalizationHelper with utility functions to perform translations for model elements => Uses LocalizationHelper in: AreaImpl ItemImpl MenuElementImpl PerspectiveImpl PartImpl WindowImpl => Updates contacts demo to provide Russian localization to the model, available via "-nl ru" runtime option => Adds ModelLifeCycleExtension and BinaryTranslatorProvider to the contacts demo to illustrate replacements of default translation service => Updates existing callers of UILabel#getLabel() and #getTooltip()
Patch "Patch alternative v.2 - model update & usage" applied to CVS Head.
I'm not sure it is a good idea to remove the local*-fields I think they had a special meaning. Eric you've been the one who added them. I also think that we should close this bug and work further in #306576 which is discussing xmi-translations
(In reply to comment #43) > I'm not sure it is a good idea to remove the local*-fields I think they had a > special meaning. Eric you've been the one who added them. > Yep, I spoke with Eric and, indeed, they there added for a different reason. Eric said he'll be re-thinking that part so he might restore fields in the future, or use a different approach to non-persisted model changes. > I also think that we should close this bug and work further in #306576 which is > discussing xmi-translations I agree. We now have an extensible translation service for the xmi elements. Things that are not there: (a) updating rendering of framework-controlled elements on language change (this would be labels on tabs) (b) updating rendering of framework contents on language change (this would be contents inside parts)
