lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-3738) Be consistent about negative vInt/vLong
Date Fri, 16 Mar 2012 19:19:39 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-3738?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231543#comment-13231543
] 

Uwe Schindler commented on LUCENE-3738:
---------------------------------------

You misunderstood my comment:

bq. I see Mike's point though: readVLong() is very general, so someone could be using it where
performance is important.

The check is only ommitted in the unrolled loop, the for-loop still contains the check. In
that case it also handles maybe too-long vints correctly, which the unrolled code will never
do. The unrolled code also has a bug, that it handles negative longs wrong, but that should
be prevented (in my opinion also for ints).

The current assert for both long and int is completely harmless, as it will only be executed,
if the vInt/vLong has the maximum number of bytes, which is very unlikely. And as said before
the check is done in the loop-based code, too. And comparison in perf showed that the speed
of the unrolled loop and the standard loop are identical, so about what are you talking?

The good thing here is that we can (in the unrolled loops) harden the check for negative vInts,
as because of the unrolled loop we have a separate cocde branch already, so we can modify
the check (which was always done before my unrolling) to do the better check.

I had the idea at that times to unroll that loop because of Java bugs, so the bug is caused
by me and I want to fix it the correct way, definitely without loosing anything. Is this so
hard to understand?
                
> Be consistent about negative vInt/vLong
> ---------------------------------------
>
>                 Key: LUCENE-3738
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3738
>             Project: Lucene - Java
>          Issue Type: Bug
>            Reporter: Michael McCandless
>             Fix For: 3.6, 4.0
>
>         Attachments: LUCENE-3738.patch, LUCENE-3738.patch, LUCENE-3738.patch
>
>
> Today, write/readVInt "allows" a negative int, in that it will encode and decode correctly,
just horribly inefficiently (5 bytes).
> However, read/writeVLong fails (trips an assert).
> I'd prefer that both vInt/vLong trip an assert if you ever try to write a negative number...
it's badly trappy today.  But, unfortunately, we sometimes rely on this... had we had this
assert in 'since the beginning' we could have avoided that.
> So, if we can't add that assert in today, I think we should at least fix readVLong to
handle negative longs... but then you quietly spend 9 bytes (even more trappy!).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message