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

CLDR Ticket #10183(accepted tools)

Opened 12 months ago

Last modified 3 weeks ago

Fix XPathParts()

Reported by: mark Owned by: mark
Component: util Data Locale:
Phase: dsub Review:
Weeks: Data Xpath:


We have a cache for XPathParts which is faster than using the constructor.

I proposed refactoring to change

x = new XPathParts();


x = XPathParts.getFrozenInstance(path).cloneAsThawed();

Now, it is even faster to remove the .cloneAsThawed(), but we can only do that if x is not going to be changed, which requires code inspection.


Change History

comment:1 Changed 11 months ago by emmons

  • Status changed from new to accepted
  • Component changed from unknown to util
  • Priority changed from assess to medium
  • Phase changed from dsub to rc
  • Milestone changed from UNSCH to 32
  • Owner changed from anybody to mark
  • Type changed from unknown to tools

comment:2 Changed 7 months ago by mark

  • Phase changed from rc to spec-beta

comment:3 Changed 6 months ago by mark

In Eclipse, use:

Introduce FactoryCreates a new factory method, which will call a selected constructor and return the created object. All references to the constructor will be replaced by calls to the new factory method.

comment:4 Changed 6 months ago by mark

Refactoring is more complicated than can do quickly. Even without accounting for the optimization to use a frozen instance, need to change:

new XPathParts()XPathParts.getInstance()
new XPathParts().set(x)XPathParts.getInstance(x)
XPathParts parts = new XPathParts(); ...; parts.set(x)... ; ... ; parts.set(x) ...;XPathParts parts; ...; parts = XPathParts.getInstance(x)... ; parts = XPathParts.getInstance(x) ...;

comment:5 Changed 6 months ago by mark

More notes for later.

The Comparator<String> attributeComparator in the following can be dropped. It is not used in the constructor, nor stored in the instance.

XPathParts(Comparator<String>, Map<String, Map<String, String>>)
XPathParts(List<Element> elements, Comparator<String> attributeComparator, Map<String, Map<String, String>> suppressionMap) {

The suppressionMap can also be removed.

It is only used in CLDRFile itself, in putRoot and write

public CLDRFile putRoot(CLDRFile rootFile) {


XPathParts parts = new XPathParts(getAttributeOrdering(), defaultSuppressionMap);

But putRoot is never used.

public CLDRFile write(PrintWriter pw, Map<String, ?> options) {


XPathParts last = new XPathParts(attributeOrdering2, defaultSuppressionMap);
XPathParts current = new XPathParts(attributeOrdering2, defaultSuppressionMap);
XPathParts lastFiltered = new XPathParts(attributeOrdering2, defaultSuppressionMap);
XPathParts currentFiltered = new XPathParts(attributeOrdering2, defaultSuppressionMap);

The only value ever used in write is defaultSuppressionMap. And that is only ever used in writing, in XPathParts.writeDifference

Rather than carry an extra field in every single XPathParts, would be better to just extract the defaultSuppressionMap from the DtdData.

Even better would be to get rid of the defaultSuppressionMap by eliminating the defaulted items as in:

<!ATTLIST dateFormat type NMTOKEN "standard" >.

The only one we have to keep is _q, but that can be hard-coded. That would cause a one-time addition of extra values to the data, but those values are used throughout all the tooling, so it would make no effective change.

comment:6 Changed 6 months ago by mark

  • Milestone changed from 32 to 33

comment:7 Changed 3 weeks ago by mark

  • Phase changed from spec-beta to dsub
  • Milestone changed from 33 to 34

Needs to be done very early in the release.


Add a comment

Modify Ticket

as accepted

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

Note: See TracTickets for help on using tickets.