You are viewing a plain text version of this content. The canonical link for it is here.
Posted to j-dev@xerces.apache.org by "Stephanie Dietzel (JIRA)" <xe...@xml.apache.org> on 2010/07/09 17:51:50 UTC
[jira] Created: (XERCESJ-1457) Forgotten calls on intern() for
QName fields
Forgotten calls on intern() for QName fields
--------------------------------------------
Key: XERCESJ-1457
URL: https://issues.apache.org/jira/browse/XERCESJ-1457
Project: Xerces2-J
Issue Type: Bug
Components: Other
Affects Versions: 2.10.0
Reporter: Stephanie Dietzel
Attachments: interning-fix.patch
In the class Qname.java, the documentation states, "To be used correctly, the strings must be identical references for equal strings."
Here are 26 places where a QName is constructed or modifed with non-interned strings.
This is an efficiency concern, because the non-interned strings may take up
more memory than interned ones. It is also a correctness concern, because
Xerces-J performs comparisons against QName fields using ==.
The attached patch corrects these problems.
For example, in XIncludeHandler.java,
new QName(
XMLSymbols.PREFIX_XMLNS,
"",
XMLSymbols.PREFIX_XMLNS + ":",
NamespaceContext.XMLNS_URI)
becomes
new QName(
XMLSymbols.PREFIX_XMLNS,
"",
(XMLSymbols.PREFIX_XMLNS + ":").intern(),
NamespaceContext.XMLNS_URI)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org
[jira] Commented: (XERCESJ-1457) Forgotten calls on intern() for
QName fields
Posted by "Michael Glavassevich (JIRA)" <xe...@xml.apache.org>.
[ https://issues.apache.org/jira/browse/XERCESJ-1457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12886754#action_12886754 ]
Michael Glavassevich commented on XERCESJ-1457:
-----------------------------------------------
Do you have any test cases which show that these proposed changes fix actual bugs? Calling intern() excessively is also an efficiency concern and isn't clear that an interned string is actually necessary in the places you've pointed out. Sometimes it's okay to break the rules when there are no consequences.
> Forgotten calls on intern() for QName fields
> --------------------------------------------
>
> Key: XERCESJ-1457
> URL: https://issues.apache.org/jira/browse/XERCESJ-1457
> Project: Xerces2-J
> Issue Type: Bug
> Components: Other
> Affects Versions: 2.10.0
> Reporter: Stephanie Dietzel
> Attachments: interning-fix.patch
>
> Original Estimate: 0.08h
> Remaining Estimate: 0.08h
>
> In the class Qname.java, the documentation states, "To be used correctly, the strings must be identical references for equal strings."
> Here are 26 places where a QName is constructed or modifed with non-interned strings.
> This is an efficiency concern, because the non-interned strings may take up
> more memory than interned ones. It is also a correctness concern, because
> Xerces-J performs comparisons against QName fields using ==.
> The attached patch corrects these problems.
> For example, in XIncludeHandler.java,
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> XMLSymbols.PREFIX_XMLNS + ":",
> NamespaceContext.XMLNS_URI)
> becomes
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> (XMLSymbols.PREFIX_XMLNS + ":").intern(),
> NamespaceContext.XMLNS_URI)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org
[jira] Commented: (XERCESJ-1457) Forgotten calls on intern() for
QName fields
Posted by "Michael Glavassevich (JIRA)" <xe...@xml.apache.org>.
[ https://issues.apache.org/jira/browse/XERCESJ-1457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12886756#action_12886756 ]
Michael Glavassevich commented on XERCESJ-1457:
-----------------------------------------------
Also note that some of these strings (e.g. the ones in the DTDGrammar) may have already been interned somewhere else.
> Forgotten calls on intern() for QName fields
> --------------------------------------------
>
> Key: XERCESJ-1457
> URL: https://issues.apache.org/jira/browse/XERCESJ-1457
> Project: Xerces2-J
> Issue Type: Bug
> Components: Other
> Affects Versions: 2.10.0
> Reporter: Stephanie Dietzel
> Attachments: interning-fix.patch
>
> Original Estimate: 0.08h
> Remaining Estimate: 0.08h
>
> In the class Qname.java, the documentation states, "To be used correctly, the strings must be identical references for equal strings."
> Here are 26 places where a QName is constructed or modifed with non-interned strings.
> This is an efficiency concern, because the non-interned strings may take up
> more memory than interned ones. It is also a correctness concern, because
> Xerces-J performs comparisons against QName fields using ==.
> The attached patch corrects these problems.
> For example, in XIncludeHandler.java,
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> XMLSymbols.PREFIX_XMLNS + ":",
> NamespaceContext.XMLNS_URI)
> becomes
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> (XMLSymbols.PREFIX_XMLNS + ":").intern(),
> NamespaceContext.XMLNS_URI)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org
[jira] Commented: (XERCESJ-1457) Forgotten calls on intern() for
QName fields
Posted by "Michael Glavassevich (JIRA)" <xe...@xml.apache.org>.
[ https://issues.apache.org/jira/browse/XERCESJ-1457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12887239#action_12887239 ]
Michael Glavassevich commented on XERCESJ-1457:
-----------------------------------------------
DTDGrammars are built from the XML scanner. The scanner interns strings (via the SymbolTable). All the components connected to it in the pipeline receive those strings. Developers who have studied Xerces' overall design [1] should have learned that. While I agree that there could be more comments in the code to help the casual reader understand I haven't seen anything in the context that these strings (and I'm also talking about the ones in XSDHandler and elsewhere) are used which would suggest that there are any actual bugs to be fixed. If you've found a real defect we should fix it, otherwise I'm inclined to return this JIRA issue.
[1] http://xerces.apache.org/xerces2-j/xni-xerces2.html
> Forgotten calls on intern() for QName fields
> --------------------------------------------
>
> Key: XERCESJ-1457
> URL: https://issues.apache.org/jira/browse/XERCESJ-1457
> Project: Xerces2-J
> Issue Type: Bug
> Components: Other
> Affects Versions: 2.10.0
> Reporter: Stephanie Dietzel
> Attachments: interning-fix.patch
>
> Original Estimate: 0.08h
> Remaining Estimate: 0.08h
>
> In the class Qname.java, the documentation states, "To be used correctly, the strings must be identical references for equal strings."
> Here are 26 places where a QName is constructed or modifed with non-interned strings.
> This is an efficiency concern, because the non-interned strings may take up
> more memory than interned ones. It is also a correctness concern, because
> Xerces-J performs comparisons against QName fields using ==.
> The attached patch corrects these problems.
> For example, in XIncludeHandler.java,
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> XMLSymbols.PREFIX_XMLNS + ":",
> NamespaceContext.XMLNS_URI)
> becomes
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> (XMLSymbols.PREFIX_XMLNS + ":").intern(),
> NamespaceContext.XMLNS_URI)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org
[jira] Commented: (XERCESJ-1457) Forgotten calls on intern() for
QName fields
Posted by "Stephanie Dietzel (JIRA)" <xe...@xml.apache.org>.
[ https://issues.apache.org/jira/browse/XERCESJ-1457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12887229#action_12887229 ]
Stephanie Dietzel commented on XERCESJ-1457:
--------------------------------------------
In XSDHandler, the QName is created with non-interned fields, and immediately passed into the method getGlobalDecl(...), which compares the QName.uri using the == operator. This control flow suggests it is not a purposeful exception to the rule and that fields must be interned for both clarity and correctness.
> Sometimes it's okay to break the rules when there are no consequences.
One consequence is readability. The QName documentation states, "To be used correctly, the strings must be identical references for equal strings." If this is not true, then could you correct the comment to clarify the circumstances in which the requirement does not hold? That will help people who are trying to understand Xerces-J and use it without introducing errors. Also, it is possible that the small cost of interning may be worth the improvement in code quality, simplicity, and avoiding future bugs.
> Also note that some of these strings (e.g. the ones in the DTDGrammar)
> may have already been interned somewhere else.
Thanks for pointing this out. Are value and otherValue always interned if they are Strings? If so, it would be really helpful to document that fact. The "value" and "otherValue" fields of of XMLContentSpec are not always interned. For example, see the call to XMLContentSpec.setValues on line 1999 of DTDGrammar.java, which sets a field to a non-interned array.
> Forgotten calls on intern() for QName fields
> --------------------------------------------
>
> Key: XERCESJ-1457
> URL: https://issues.apache.org/jira/browse/XERCESJ-1457
> Project: Xerces2-J
> Issue Type: Bug
> Components: Other
> Affects Versions: 2.10.0
> Reporter: Stephanie Dietzel
> Attachments: interning-fix.patch
>
> Original Estimate: 0.08h
> Remaining Estimate: 0.08h
>
> In the class Qname.java, the documentation states, "To be used correctly, the strings must be identical references for equal strings."
> Here are 26 places where a QName is constructed or modifed with non-interned strings.
> This is an efficiency concern, because the non-interned strings may take up
> more memory than interned ones. It is also a correctness concern, because
> Xerces-J performs comparisons against QName fields using ==.
> The attached patch corrects these problems.
> For example, in XIncludeHandler.java,
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> XMLSymbols.PREFIX_XMLNS + ":",
> NamespaceContext.XMLNS_URI)
> becomes
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> (XMLSymbols.PREFIX_XMLNS + ":").intern(),
> NamespaceContext.XMLNS_URI)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org
[jira] Updated: (XERCESJ-1457) Forgotten calls on intern() for
QName fields
Posted by "Stephanie Dietzel (JIRA)" <xe...@xml.apache.org>.
[ https://issues.apache.org/jira/browse/XERCESJ-1457?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Stephanie Dietzel updated XERCESJ-1457:
---------------------------------------
Attachment: interning-fix.patch
> Forgotten calls on intern() for QName fields
> --------------------------------------------
>
> Key: XERCESJ-1457
> URL: https://issues.apache.org/jira/browse/XERCESJ-1457
> Project: Xerces2-J
> Issue Type: Bug
> Components: Other
> Affects Versions: 2.10.0
> Reporter: Stephanie Dietzel
> Attachments: interning-fix.patch
>
> Original Estimate: 0.08h
> Remaining Estimate: 0.08h
>
> In the class Qname.java, the documentation states, "To be used correctly, the strings must be identical references for equal strings."
> Here are 26 places where a QName is constructed or modifed with non-interned strings.
> This is an efficiency concern, because the non-interned strings may take up
> more memory than interned ones. It is also a correctness concern, because
> Xerces-J performs comparisons against QName fields using ==.
> The attached patch corrects these problems.
> For example, in XIncludeHandler.java,
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> XMLSymbols.PREFIX_XMLNS + ":",
> NamespaceContext.XMLNS_URI)
> becomes
> new QName(
> XMLSymbols.PREFIX_XMLNS,
> "",
> (XMLSymbols.PREFIX_XMLNS + ":").intern(),
> NamespaceContext.XMLNS_URI)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: j-dev-help@xerces.apache.org