You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by "Greg Brown (JIRA)" <ji...@apache.org> on 2009/07/30 22:40:14 UTC
[jira] Created: (PIVOT-198) Create ImmutablePath class; update
TreeView to use ImmutableList in getSelectedPaths()
Create ImmutablePath class; update TreeView to use ImmutableList in getSelectedPaths()
--------------------------------------------------------------------------------------
Key: PIVOT-198
URL: https://issues.apache.org/jira/browse/PIVOT-198
Project: Pivot
Issue Type: Improvement
Components: wtk
Affects Versions: 1.3
Reporter: Greg Brown
Assignee: Todd Volkert
Priority: Minor
Fix For: 1.3
Create an ImmutablePath class that extends Path; use these internally in TreeView so we can return an ImmutableList wrapper around the selected paths and avoid the deep copy that we currently perform.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (PIVOT-198) Create ImmutablePath class; update
TreeView to use ImmutableList in getSelectedPaths()
Posted by "Todd Volkert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIVOT-198?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Todd Volkert resolved PIVOT-198.
--------------------------------
Resolution: Fixed
> Create ImmutablePath class; update TreeView to use ImmutableList in getSelectedPaths()
> --------------------------------------------------------------------------------------
>
> Key: PIVOT-198
> URL: https://issues.apache.org/jira/browse/PIVOT-198
> Project: Pivot
> Issue Type: Improvement
> Components: wtk
> Affects Versions: 1.3
> Reporter: Greg Brown
> Assignee: Todd Volkert
> Priority: Minor
> Fix For: 1.3
>
>
> Create an ImmutablePath class that extends Path; use these internally in TreeView so we can return an ImmutableList wrapper around the selected paths and avoid the deep copy that we currently perform.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIVOT-198) Create ImmutablePath class; update
TreeView to use ImmutableList in getSelectedPaths()
Posted by "Todd Volkert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIVOT-198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743074#action_12743074 ]
Todd Volkert commented on PIVOT-198:
------------------------------------
After looking back at TreeView, I think that we should keep it as is. We have to update the internal Path instances when we receive data events. For instance, we decrement path indexes when a preceding path element is removed from the list. As such, we can't use ImmutablePath instances internally the way the code is currently structured. We could choose to use ImmutablePath instances internally if we created new Paths instead of updating the existing paths, but it's not clear (and is unlikely) that such a method would be more efficient than the current method. I suggest renaming this ticket "Create ImmutablePath class" and closing it.
> Create ImmutablePath class; update TreeView to use ImmutableList in getSelectedPaths()
> --------------------------------------------------------------------------------------
>
> Key: PIVOT-198
> URL: https://issues.apache.org/jira/browse/PIVOT-198
> Project: Pivot
> Issue Type: Improvement
> Components: wtk
> Affects Versions: 1.3
> Reporter: Greg Brown
> Assignee: Todd Volkert
> Priority: Minor
> Fix For: 1.3
>
>
> Create an ImmutablePath class that extends Path; use these internally in TreeView so we can return an ImmutableList wrapper around the selected paths and avoid the deep copy that we currently perform.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIVOT-198) Create ImmutablePath class; update
TreeView to use ImmutableList in getSelectedPaths()
Posted by "Greg Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIVOT-198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12738569#action_12738569 ]
Greg Brown commented on PIVOT-198:
----------------------------------
The ImmutablePath class has been created.
> Create ImmutablePath class; update TreeView to use ImmutableList in getSelectedPaths()
> --------------------------------------------------------------------------------------
>
> Key: PIVOT-198
> URL: https://issues.apache.org/jira/browse/PIVOT-198
> Project: Pivot
> Issue Type: Improvement
> Components: wtk
> Affects Versions: 1.3
> Reporter: Greg Brown
> Assignee: Todd Volkert
> Priority: Minor
> Fix For: 1.3
>
>
> Create an ImmutablePath class that extends Path; use these internally in TreeView so we can return an ImmutableList wrapper around the selected paths and avoid the deep copy that we currently perform.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIVOT-198) Create ImmutablePath class; update
TreeView to use ImmutableList in getSelectedPaths()
Posted by "Greg Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIVOT-198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743193#action_12743193 ]
Greg Brown commented on PIVOT-198:
----------------------------------
I just ran a test to see what kind of numbers we are looking at here. For sure, updating individual path elements is faster. However, the gain would be unnoticeable to most users.
For 1,000 simulated selected paths, in the worst-case scenario of updating all of them, the numbers were as follows (averaged over 10,000 test runs):
Update: 0.0342ms; new: 0.2818ms
So, updating 1,000 paths took 0.03ms, whereas replacing them took 0.28ms. Neither one is going to be noticeable to the user, and 1,000 items is a pretty extreme case. Also, this performance hit is only incurred when tree data is actually modified when a large number of items are selected - this isn't a situation we're likely to run into very often.
On the other hand, asking for a tree view's current selection may happen fairly often. Here's the results of copying the entire path structure vs. returning an immutable list:
Copy: 0.2521ms; get: 0.0008ms
Either way you look at it, the numbers are negligible. However, since the performance penalty of updating the selection is only paid when the model changes under an existing selection, and callers may often be asking for the current selection state, I would suggest that using immutable paths is the better alternative.
> Create ImmutablePath class; update TreeView to use ImmutableList in getSelectedPaths()
> --------------------------------------------------------------------------------------
>
> Key: PIVOT-198
> URL: https://issues.apache.org/jira/browse/PIVOT-198
> Project: Pivot
> Issue Type: Improvement
> Components: wtk
> Affects Versions: 1.3
> Reporter: Greg Brown
> Assignee: Todd Volkert
> Priority: Minor
> Fix For: 1.3
>
>
> Create an ImmutablePath class that extends Path; use these internally in TreeView so we can return an ImmutableList wrapper around the selected paths and avoid the deep copy that we currently perform.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIVOT-198) Create ImmutablePath class; update
TreeView to use ImmutableList in getSelectedPaths()
Posted by "Greg Brown (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIVOT-198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743182#action_12743182 ]
Greg Brown commented on PIVOT-198:
----------------------------------
We have to do the same thing with Spans in ListView and TableView, since Spans are also immutable. That's the tradeoff I mentioned when I originally suggested this change. My guess is that the performance overhead of creating a new span or path vs. updating an existing one is negligible. So I still think it is worth doing. Also, this is even less of an issue for TreeView than ListView or TableView, since it is far less likely to have multiple nodes selected.
> Create ImmutablePath class; update TreeView to use ImmutableList in getSelectedPaths()
> --------------------------------------------------------------------------------------
>
> Key: PIVOT-198
> URL: https://issues.apache.org/jira/browse/PIVOT-198
> Project: Pivot
> Issue Type: Improvement
> Components: wtk
> Affects Versions: 1.3
> Reporter: Greg Brown
> Assignee: Todd Volkert
> Priority: Minor
> Fix For: 1.3
>
>
> Create an ImmutablePath class that extends Path; use these internally in TreeView so we can return an ImmutableList wrapper around the selected paths and avoid the deep copy that we currently perform.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.