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

CLDR Ticket #10183(accepted tools)

Opened 6 months ago

Last modified 3 weeks ago

Fix XPathParts()

Reported by: mark Owned by: mark
Component: util Data Locale:
Phase: spec-beta Review:
Weeks: Data Xpath:
Xref:

Description

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

I proposed refactoring to change

x = new XPathParts();

to

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.

Attachments

Change History

comment:1 Changed 6 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 6 weeks ago by mark

  • Phase changed from rc to spec-beta

comment:3 Changed 3 weeks 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 3 weeks 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:

fromto
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 3 weeks 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 3 weeks ago by mark

  • Milestone changed from 32 to 33
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.