You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Eks Dev (JIRA)" <ji...@apache.org> on 2008/03/11 10:24:48 UTC
[jira] Created: (LUCENE-1217) use isBinary cached variable instead
of instanceof in Filed
use isBinary cached variable instead of instanceof in Filed
-----------------------------------------------------------
Key: LUCENE-1217
URL: https://issues.apache.org/jira/browse/LUCENE-1217
Project: Lucene - Java
Issue Type: Improvement
Components: Other
Reporter: Eks Dev
Priority: Trivial
Filed class can hold three types of values,
See: AbstractField.java protected Object fieldsData = null;
currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
This patch makes consistent use of cached variable isBinary.
Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
Thinking aloud:
Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
code:java{
public static final interface Type{
public static final byte BOOLEAN = 0;
public static final byte STRING = 1;
public static final byte READER = 2;
....
}
}
and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-1217) use isBinary cached variable
instead of instanceof in Filed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12577637#action_12577637 ]
Michael McCandless commented on LUCENE-1217:
--------------------------------------------
OK the new patch passes all tests -- thanks!
One unrelated thing I noticed: it looks like you can get a binary LazyField and then ask for its stringValue(), and vice-versa. Ie we are failing to check in binaryValue() that the field is in fact binary even though when we create the LazyField we know whether it is. I'll open a separate issue for this.
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: Lucene-1217-take1.patch, LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-1217) use isBinary cached variable
instead of instanceof in Filed
Posted by "Eks Dev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12577591#action_12577591 ]
Eks Dev commented on LUCENE-1217:
---------------------------------
thanks fof looking into it!
Subclassing now with backwards compatibility would be clumsy, I was thinking about it but could not find clean way to make it.
>>Or we could wait until Java 5 (3.0) and use real enums?
yes, that is ultimate solution, but my line of thoughts was that "poor man's enum"->java 5 enum migration would be trivial later... but do not change working code kicks-in here :)
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-1217) use isBinary cached variable
instead of instanceof in Filed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12577598#action_12577598 ]
Michael McCandless commented on LUCENE-1217:
--------------------------------------------
Actually seeing a test failure with this:
[junit] Testcase: testLazyFields(org.apache.lucene.index.TestFieldsReader): FAILED
[junit] bytes is null and it shouldn't be
[junit] junit.framework.AssertionFailedError: bytes is null and it shouldn't be
[junit] at org.apache.lucene.index.TestFieldsReader.testLazyFields(TestFieldsReader.java:132)
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-1217) use isBinary cached variable instead
of instanceof in Filed
Posted by "Eks Dev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eks Dev updated LUCENE-1217:
----------------------------
Attachment: LUCENE-1217.patch
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Priority: Trivial
> Attachments: LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-1217) use isBinary cached variable instead
of instanceof in Field
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated LUCENE-1217:
---------------------------------
Description:
Field class can hold three types of values,
See: AbstractField.java protected Object fieldsData = null;
currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
This patch makes consistent use of cached variable isBinary.
Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
Thinking aloud:
Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
code:java{
public static final interface Type{
public static final byte BOOLEAN = 0;
public static final byte STRING = 1;
public static final byte READER = 2;
....
}
}
and use that instead of isBinary + instanceof?
was:
Filed class can hold three types of values,
See: AbstractField.java protected Object fieldsData = null;
currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
This patch makes consistent use of cached variable isBinary.
Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
Thinking aloud:
Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
code:java{
public static final interface Type{
public static final byte BOOLEAN = 0;
public static final byte STRING = 1;
public static final byte READER = 2;
....
}
}
and use that instead of isBinary + instanceof?
Lucene Fields: [New, Patch Available] (was: [Patch Available, New])
Summary: use isBinary cached variable instead of instanceof in Field (was: use isBinary cached variable instead of instanceof in Filed)
fix typo that's been bugging me
> use isBinary cached variable instead of instanceof in Field
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: Lucene-1217-take1.patch, LUCENE-1217.patch
>
>
> Field class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Resolved: (LUCENE-1217) use isBinary cached variable instead
of instanceof in Filed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless resolved LUCENE-1217.
----------------------------------------
Resolution: Fixed
Lucene Fields: [New, Patch Available] (was: [Patch Available, New])
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: Lucene-1217-take1.patch, LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Assigned: (LUCENE-1217) use isBinary cached variable instead
of instanceof in Filed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael McCandless reassigned LUCENE-1217:
------------------------------------------
Assignee: Michael McCandless
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-1217) use isBinary cached variable
instead of instanceof in Filed
Posted by "Michael McCandless (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12577547#action_12577547 ]
Michael McCandless commented on LUCENE-1217:
--------------------------------------------
Patch looks good. I will commit shortly. Thanks Eks Dev.
{quote}
Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
{quote}
Or we could wait until Java 5 (3.0) and use real enums?
Or ... maybe we should have subclasses of Field (TextField, BinaryField,
ReaderField, TokenStreamField) which override the corresponding method
(and the base Field.java would still implement these methods but
return null)? Though this would be a rather large change...
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-1217) use isBinary cached variable
instead of instanceof in Filed
Posted by "Eks Dev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12577601#action_12577601 ]
Eks Dev commented on LUCENE-1217:
---------------------------------
hah, this bug just justified this patch :)
sorry, I should have run tests before... nothing is trivial enough.
The problem was indeed isBinary that went out of sync in LazyField, new patch follows
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-1217) use isBinary cached variable instead
of instanceof in Filed
Posted by "Eks Dev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-1217?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eks Dev updated LUCENE-1217:
----------------------------
Attachment: Lucene-1217-take1.patch
new patch, fixes isBinary status in LazyField
> use isBinary cached variable instead of instanceof in Filed
> -----------------------------------------------------------
>
> Key: LUCENE-1217
> URL: https://issues.apache.org/jira/browse/LUCENE-1217
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Other
> Reporter: Eks Dev
> Assignee: Michael McCandless
> Priority: Trivial
> Attachments: Lucene-1217-take1.patch, LUCENE-1217.patch
>
>
> Filed class can hold three types of values,
> See: AbstractField.java protected Object fieldsData = null;
> currently, mainly RTTI (instanceof) is used to determine the type of the value stored in particular instance of the Field, but for binary value we have mixed RTTI and cached variable "boolean isBinary"
> This patch makes consistent use of cached variable isBinary.
> Benefit: consistent usage of method to determine run-time type for binary case (reduces chance to get out of sync on cached variable). It should be slightly faster as well.
> Thinking aloud:
> Would it not make sense to maintain type with some integer/byte"poor man's enum" (Interface with a couple of constants)
> code:java{
> public static final interface Type{
> public static final byte BOOLEAN = 0;
> public static final byte STRING = 1;
> public static final byte READER = 2;
> ....
> }
> }
> and use that instead of isBinary + instanceof?
--
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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org