You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Edward Ribeiro (JIRA)" <ji...@apache.org> on 2009/07/04 18:22:47 UTC

[jira] Created: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Replace Map.keySet by more efficient Map.entrySet
-------------------------------------------------

                 Key: CASSANDRA-275
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
             Project: Cassandra
          Issue Type: Improvement
            Reporter: Edward Ribeiro
            Priority: Minor



When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 

I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Michael Greene (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728407#action_12728407 ] 

Michael Greene commented on CASSANDRA-275:
------------------------------------------

The freezing/defreezing of maps in RowMutation is incorrect in the patch, so that should be left out if portions are committed.

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Edward Ribeiro (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728410#action_12728410 ] 

Edward Ribeiro commented on CASSANDRA-275:
------------------------------------------

Really. Well, I propose to get rid of this patch altogether, but my previous comment is still valid.

Some places of the code base look pretty bad. Anyone could get in charge of fixing these in the future?

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Ellis resolved CASSANDRA-275.
--------------------------------------

    Resolution: Incomplete

ok, closed

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Edward Ribeiro (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Edward Ribeiro updated CASSANDRA-275:
-------------------------------------

    Attachment: CASSANDRA-275.patch

The patch is not huge but affects many classes, so a slower review of its correctness is highly recommended. 

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Edward Ribeiro (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728400#action_12728400 ] 

Edward Ribeiro commented on CASSANDRA-275:
------------------------------------------

Yes, I really forgot to use code conventions this time, but this can be fixed with another patch. Sorry for that.  I agree that the code with entrySet looks a bit uglier, but it's just a matter of getting used to it.

Well, the patch can be thrown out, but there are some places where the code cries for better coding, like the following (Gossiper class):

        Map<String, ApplicationState> appStateMap = epState.getApplicationState();
        Set<String> keys = appStateMap.keySet();
        for ( String key : keys )
        {
            int stateVersion = appStateMap.get(key).getStateVersion();
             versions.add( stateVersion );
         }

Well, if it's iterating over all the values of the map why not just do the following?

        Map<String, ApplicationState> appStateMap = epState.getApplicationState();
        for (ApplicationState appState : appStateMap.values())
          {
            int stateVersion = appState.getStateVersion();
            versions.add( stateVersion );
         }

As I said before, this patch can be thrown out, but these wtf code snippets as the one above, that was fixed by this patch, should still be the target of better coding for the benefit of the long term Cassandra maintenance. 


> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728419#action_12728419 ] 

Jonathan Ellis commented on CASSANDRA-275:
------------------------------------------

Sure, if only the values are being accessed then +1 iterating over .values().

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12736284#action_12736284 ] 

Jonathan Ellis commented on CASSANDRA-275:
------------------------------------------

is this going anywhere?

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728136#action_12728136 ] 

Jonathan Ellis commented on CASSANDRA-275:
------------------------------------------

Minor problem: this patch does not follow Cassandra code conventions

Bigger problem: using entrySet is uglier than iterating keys and calling get in many situations.  I'm -1 on doing a blanket replace in the name of efficiency.

> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-275) Replace Map.keySet by more efficient Map.entrySet

Posted by "Edward Ribeiro (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737781#action_12737781 ] 

Edward Ribeiro commented on CASSANDRA-275:
------------------------------------------

As far as I remember, this issue should be closed for now and future improvements on Map looping should be done on a case by case basis. 



> Replace Map.keySet by more efficient Map.entrySet
> -------------------------------------------------
>
>                 Key: CASSANDRA-275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-275
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Edward Ribeiro
>            Priority: Minor
>         Attachments: CASSANDRA-275.patch
>
>
> When you iterates over all key-values of a Map is better to use entrySet() instead of a call to entryKey() and a call to get() inside the loop. It's more efficient. 
> I've seen a patch like this before, but I don't think it was applied at all. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.