[Unicode]   Common Locale Data Repository : Bug Tracking Home | Site Map | Search
 
Modify

CLDR Ticket #7428(accepted tools)

Opened 3 years ago

Last modified 21 months ago

Freeze collators (only trunk change was reverted)

Reported by: mark Owned by: anybody
Component: util Data Locale:
Phase: dvet Review:
Weeks: Data Xpath:
Xref:

Description (last modified by mark) (diff)

Collators are not thread-safe unless they are frozen.

When they are frozen, then the compare method then becomes thread-safe (at a slight cost).

So it would be cleaner to make all of the collators that are built once and then used be frozen. We should check for all instances of Collator and RuleBasedCollator in the code base.


The only trunk change was reverted, so it is ok for this to be pushed to 27.

Attachments

Change History

comment:1 Changed 3 years ago by mark

  • Owner changed from anybody to ribnitz
  • Priority changed from assess to major
  • Status changed from new to assigned
  • Milestone changed from UNSCH to 26dsub

comment:2 Changed 3 years ago by ribnitz

  • Status changed from assigned to reviewing
  • Review set to mark

Also includes minor fixes to tooling to create the output directory that is being written to; because of item 7452 CountItems/ShowZoneEquivalences is not included in these changes.

comment:3 Changed 3 years ago by mark

  • Status changed from reviewing to accepted

http://unicode.org/cldr/trac/changeset/10334

Removes the line:

UCA_BASE.setNumericCollation(true);

That must *not* be done, it is significant.

Set<String> nameAndInfo = new TreeSet<String>(info.getCollator().freeze());

Unless info has a mutable collator, this should be done in info, not here.

private Map<String, Object> compareMap = new TreeMap<String, Object>(getDefaultCollation().freeze());

Same here. Unless the collator in getDefaultCollation() must be mutable, freeze there.

http://unicode.org/cldr/trac/changeset/10349

private static class ThawingCollatorWrapper<E extends Collator> implements Cloneable {

Don't understand what this is doing. cloneAsThawed should product an unfrozen clone already.

http://unicode.org/cldr/trac/changeset/10353

private static void assertDirWritable(String directory) throws IOException {

Don't copy code; put in one place in /util/. I think we already have a FileUtilities there.

comment:4 Changed 3 years ago by mark

  • Status changed from accepted to assigned

comment:5 Changed 3 years ago by mark

  • Owner changed from ribnitz to googler

comment:6 Changed 3 years ago by mark

  • Owner changed from googler to mark

comment:7 Changed 3 years ago by mark

  • Component changed from unknown to tools

comment:8 Changed 3 years ago by mark

  • Summary changed from Freeze collators to Freeze collators (only trunk change was reverted)
  • Description modified (diff)
  • Milestone changed from 26dsub to 27dvet

comment:9 Changed 3 years ago by markus

  • Phase set to dvet
  • Milestone changed from 27dvet to 27

comment:10 Changed 2 years ago by mark

  • Keywords working added

comment:11 Changed 2 years ago by mark

  • Phase changed from dvet to dsub
  • Milestone changed from 27 to 28

comment:12 Changed 2 years ago by mark

  • Phase changed from dsub to dvet

comment:13 Changed 2 years ago by markus

  • Type changed from unknown to tools
  • Component changed from tools to unknown

comment:14 Changed 2 years ago by srl

  • Status changed from assigned to accepted

comment:15 Changed 22 months ago by emmons

  • Component changed from unknown to util

comment:16 Changed 22 months ago by mark

We create a gazillion collators.

Concluded that the best approach would be:

  1. Replace all Collator.getInstance() methods with Collator.getInstance(ULocale.Root)
  2. Replace all instances of Collator.getInstance(ULocale.English) with Collator.getInstance(ULocale.Root).
    • But check each one just in case.
  3. Replace each instance of Collator.getInstance(ULocale.Root) with Collator.getInstance(ULocale.Root).freeze()
    • But check each instance to see if the freeze needs to be moved after a setter, like setNumeric(...)
  4. Add a new static methods to CLDRConfig for getting
    1. the Root collator (frozen)
    2. the Root collator set to numeric (frozen)
  5. Replace Collator.getInstance(ULocale.Root).freeze() by a call to the first, and ones with numeric by the second.
  6. See if there's any other centralization we can do.

comment:17 Changed 22 months ago by mark

  • Owner changed from mark to anybody
  • Review mark deleted

comment:18 Changed 21 months ago by emmons

  • Milestone changed from 28 to 28roll

Moving all outstanding 28 tickets to 28roll. We will discuss disposition of these at the next CLDR TC.

View

Add a comment

Modify Ticket

Action
as accepted
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.