[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[ https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15182414#comment-15182414 ] Stefania commented on CASSANDRA-10750: -- 3.2. Should we ninja fix CHANGES.txt? > Minor code improvements > --- > > Key: CASSANDRA-10750 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10750 > Project: Cassandra > Issue Type: Improvement >Reporter: Robert Stupp >Assignee: Robert Stupp >Priority: Minor > Fix For: 3.2 > > > Went though several IDE inspections and found some places in the code that > could be improved. These are just minor improvements and no bug fixes (except > one minor "theoretical" thing). > The [branch on github against > trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains > a series of commits: > * simplify Mutation.apply to remove the casts > * "minor code improvements" just replaces some expressions that are > effectively constant > * remove unused assignments (probably just cosmetic) > * collapse identical if-branches (probably just cosmetic) > * empty array constants > * fix printf usage (could potentially raise an exception in printf) > * replace tail-recursion in some critical sections (as the JVM cannot > optimize that AFAIK) > * remove methods identical to their super methods (probably just cosmetic) > [cassci results > here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[ https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15179981#comment-15179981 ] Sylvain Lebresne commented on CASSANDRA-10750: -- Can you please dig up in which release this made it in for history sake? > Minor code improvements > --- > > Key: CASSANDRA-10750 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10750 > Project: Cassandra > Issue Type: Improvement >Reporter: Robert Stupp >Assignee: Robert Stupp >Priority: Minor > > Went though several IDE inspections and found some places in the code that > could be improved. These are just minor improvements and no bug fixes (except > one minor "theoretical" thing). > The [branch on github against > trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains > a series of commits: > * simplify Mutation.apply to remove the casts > * "minor code improvements" just replaces some expressions that are > effectively constant > * remove unused assignments (probably just cosmetic) > * collapse identical if-branches (probably just cosmetic) > * empty array constants > * fix printf usage (could potentially raise an exception in printf) > * replace tail-recursion in some critical sections (as the JVM cannot > optimize that AFAIK) > * remove methods identical to their super methods (probably just cosmetic) > [cassci results > here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[
https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15063942#comment-15063942
]
Stefania commented on CASSANDRA-10750:
--
CI looks good now, the failing {{cqlsh_copy_tests}} are because we need to
rebase on CASSANDRA-9302, since it has been committed the dtests have been
updated and they fail without the latest cqlsh code, totally unrelated.
+1 for commit.
> Minor code improvements
> ---
>
> Key: CASSANDRA-10750
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10750
> Project: Cassandra
> Issue Type: Improvement
>Reporter: Robert Stupp
>Assignee: Robert Stupp
>Priority: Minor
>
> Went though several IDE inspections and found some places in the code that
> could be improved. These are just minor improvements and no bug fixes (except
> one minor "theoretical" thing).
> The [branch on github against
> trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains
> a series of commits:
> * simplify Mutation.apply to remove the casts
> * "minor code improvements" just replaces some expressions that are
> effectively constant
> * remove unused assignments (probably just cosmetic)
> * collapse identical if-branches (probably just cosmetic)
> * empty array constants
> * fix printf usage (could potentially raise an exception in printf)
> * replace tail-recursion in some critical sections (as the JVM cannot
> optimize that AFAIK)
> * remove methods identical to their super methods (probably just cosmetic)
> [cassci results
> here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[ https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15058265#comment-15058265 ] Robert Stupp commented on CASSANDRA-10750: -- Your changes LGTM and I've cherry-picked them onto my branch. Also removed the tail-recursion changes in stress - I'd like to leave the other changes as these are in "hot" code paths. Triggered another CI run. Not sure whether we can get a clean run (without timeouts). It not, I can run these tests locally. > Minor code improvements > --- > > Key: CASSANDRA-10750 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10750 > Project: Cassandra > Issue Type: Improvement >Reporter: Robert Stupp >Assignee: Robert Stupp >Priority: Minor > > Went though several IDE inspections and found some places in the code that > could be improved. These are just minor improvements and no bug fixes (except > one minor "theoretical" thing). > The [branch on github against > trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains > a series of commits: > * simplify Mutation.apply to remove the casts > * "minor code improvements" just replaces some expressions that are > effectively constant > * remove unused assignments (probably just cosmetic) > * collapse identical if-branches (probably just cosmetic) > * empty array constants > * fix printf usage (could potentially raise an exception in printf) > * replace tail-recursion in some critical sections (as the JVM cannot > optimize that AFAIK) > * remove methods identical to their super methods (probably just cosmetic) > [cassci results > here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[ https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15058313#comment-15058313 ] Stefania commented on CASSANDRA-10750: -- bq. Also removed the tail-recursion changes in stress - I'd like to leave the other changes as these are in "hot" code paths. WFM +1 once CI is OK. > Minor code improvements > --- > > Key: CASSANDRA-10750 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10750 > Project: Cassandra > Issue Type: Improvement >Reporter: Robert Stupp >Assignee: Robert Stupp >Priority: Minor > > Went though several IDE inspections and found some places in the code that > could be improved. These are just minor improvements and no bug fixes (except > one minor "theoretical" thing). > The [branch on github against > trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains > a series of commits: > * simplify Mutation.apply to remove the casts > * "minor code improvements" just replaces some expressions that are > effectively constant > * remove unused assignments (probably just cosmetic) > * collapse identical if-branches (probably just cosmetic) > * empty array constants > * fix printf usage (could potentially raise an exception in printf) > * replace tail-recursion in some critical sections (as the JVM cannot > optimize that AFAIK) > * remove methods identical to their super methods (probably just cosmetic) > [cassci results > here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[
https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15057618#comment-15057618
]
Robert Stupp commented on CASSANDRA-10750:
--
Yea - {{Stack.java}} produced NPEs because the order of the constants (static
final fields) was wrong.
After that fix, both utests and dtests look ok - i.e. there are "just" timeouts
in the dtests similar to current trunk.
> Minor code improvements
> ---
>
> Key: CASSANDRA-10750
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10750
> Project: Cassandra
> Issue Type: Improvement
>Reporter: Robert Stupp
>Assignee: Robert Stupp
>Priority: Minor
>
> Went though several IDE inspections and found some places in the code that
> could be improved. These are just minor improvements and no bug fixes (except
> one minor "theoretical" thing).
> The [branch on github against
> trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains
> a series of commits:
> * simplify Mutation.apply to remove the casts
> * "minor code improvements" just replaces some expressions that are
> effectively constant
> * remove unused assignments (probably just cosmetic)
> * collapse identical if-branches (probably just cosmetic)
> * empty array constants
> * fix printf usage (could potentially raise an exception in printf)
> * replace tail-recursion in some critical sections (as the JVM cannot
> optimize that AFAIK)
> * remove methods identical to their super methods (probably just cosmetic)
> [cassci results
> here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[
https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15057691#comment-15057691
]
Stefania commented on CASSANDRA-10750:
--
Well spotted.
I've finished the review, I've put my changes in [this
commit|https://github.com/stef1927/cassandra/commit/fd56e6a8361e0c6a6bdd90fc486dd482a2245c01]
of [this branch|https://github.com/stef1927/cassandra/commits/10750]. I've
only changed slightly {{PasswordAuthenticator}} and {{MerkleTree}}, the rest
should be nits.
The replacement of recursive code with iterative code, whilst it's true that it
is more efficient, it is also true that the recursive code is more elegant. I
am thinking of the 2 changes in the stress tool specifically, they don't seem
to recurse much and the original code was more elegant. I leave that to your
decision however.
If we can get one clean CI run without timeouts and with my changes cross
reviewed by you and applied, then this can be committed.
> Minor code improvements
> ---
>
> Key: CASSANDRA-10750
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10750
> Project: Cassandra
> Issue Type: Improvement
>Reporter: Robert Stupp
>Assignee: Robert Stupp
>Priority: Minor
>
> Went though several IDE inspections and found some places in the code that
> could be improved. These are just minor improvements and no bug fixes (except
> one minor "theoretical" thing).
> The [branch on github against
> trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains
> a series of commits:
> * simplify Mutation.apply to remove the casts
> * "minor code improvements" just replaces some expressions that are
> effectively constant
> * remove unused assignments (probably just cosmetic)
> * collapse identical if-branches (probably just cosmetic)
> * empty array constants
> * fix printf usage (could potentially raise an exception in printf)
> * replace tail-recursion in some critical sections (as the JVM cannot
> optimize that AFAIK)
> * remove methods identical to their super methods (probably just cosmetic)
> [cassci results
> here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10750) Minor code improvements
[ https://issues.apache.org/jira/browse/CASSANDRA-10750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15056159#comment-15056159 ] Stefania commented on CASSANDRA-10750: -- [~snazy] : I have just started working on the review and I hope to have some more comments by tomorrow. In the meantime you may want to rebase and take a look at the cassci results as they don't look too good. Aside from the usual dtests time outs, there seems to be some NPEs. The utests also had a bad run, at a minimum run them one more time to make sure the additional time outs are unrelated. > Minor code improvements > --- > > Key: CASSANDRA-10750 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10750 > Project: Cassandra > Issue Type: Improvement >Reporter: Robert Stupp >Assignee: Robert Stupp >Priority: Minor > > Went though several IDE inspections and found some places in the code that > could be improved. These are just minor improvements and no bug fixes (except > one minor "theoretical" thing). > The [branch on github against > trunk|https://github.com/snazy/cassandra/tree/10750-code-opts-trunk] contains > a series of commits: > * simplify Mutation.apply to remove the casts > * "minor code improvements" just replaces some expressions that are > effectively constant > * remove unused assignments (probably just cosmetic) > * collapse identical if-branches (probably just cosmetic) > * empty array constants > * fix printf usage (could potentially raise an exception in printf) > * replace tail-recursion in some critical sections (as the JVM cannot > optimize that AFAIK) > * remove methods identical to their super methods (probably just cosmetic) > [cassci results > here|http://cassci.datastax.com/view/Dev/view/snazy/search/?q=snazy-10750-] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
