You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Michael Mikhulya (JIRA)" <ji...@apache.org> on 2009/12/15 16:22:18 UTC
[jira] Created: (WICKET-2618) Don't call IDataProvider.size method
when IDataProvider.iterator(first, count) returned the number of elements
less than "count"
Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
--------------------------------------------------------------------------------------------------------------------------------
Key: WICKET-2618
URL: https://issues.apache.org/jira/browse/WICKET-2618
Project: Wicket
Issue Type: Improvement
Components: wicket
Affects Versions: 1.4.4
Reporter: Michael Mikhulya
IDataProvider has two mehtods:
Iterator<? extends T> iterator(int first, int count);
int size()
which in many implementations do access to database.
Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
In some cases it is possible to avoid call to IDataProvider.size method.
For example when "iterator" returned the number of elements less than acquired "count".
This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-2618) Don't call IDataProvider.size
method when IDataProvider.iterator(first, count) returned the number of
elements less than "count"
Posted by "Johan Compagner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793977#action_12793977 ]
Johan Compagner commented on WICKET-2618:
-----------------------------------------
i think there will be a small problem with this
and we discussed this loads of times already...
the 2 methods we have:
Iterator<? extends T> iterator(int first, int count);
int size()
are called in this order if i am not mistaken:
int size()
Iterator<? extends T> iterator(int first, int count);
because the result of size() is used as for the parameter count...
But if you are able to extract it just from the list. Then size should just call your list and return that size and that list is then used fo give the iterator back in the iterator call.
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-2618) Don't call IDataProvider.size
method when IDataProvider.iterator(first, count) returned the number of
elements less than "count"
Posted by "Juergen Donnerstag (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793969#action_12793969 ]
Juergen Donnerstag commented on WICKET-2618:
--------------------------------------------
Since 1.4.x releases must remain backwards compatible, no changes to the API are permitted. It'll be considered for 1.5 than.
Question on your application: I assume you are not using something like <first> <prev> 1 2 3 4 5 ... 10 <next> <last> in your app, since for that to work you'll obviously need the size. Please consider that with your changes since the implementation has to work many use cases.
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-2618) Don't call IDataProvider.size
method when IDataProvider.iterator(first, count) returned the number of
elements less than "count"
Posted by "Michael Mikhulya (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793965#action_12793965 ]
Michael Mikhulya commented on WICKET-2618:
------------------------------------------
I'll do necessary changes and prepare a patch on weekend.
But there is one problem with proposed solution - it is not enough to provide a decorator, since optimization is possible only when client first call an "iterator" method.
So I need to change clients (DataViewBase, DataTable, ...).
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-2618) Don't call IDataProvider.size
method when IDataProvider.iterator(first, count) returned the number of
elements less than "count"
Posted by "Juergen Donnerstag (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793665#action_12793665 ]
Juergen Donnerstag commented on WICKET-2618:
--------------------------------------------
I'm happy to look at your implementation. Please attach a patch file. Thanks
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (WICKET-2618) Don't call
IDataProvider.size method when IDataProvider.iterator(first, count)
returned the number of elements less than "count"
Posted by "Michael Mikhulya (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12801771#action_12801771 ]
Michael Mikhulya edited comment on WICKET-2618 at 1/18/10 1:09 PM:
-------------------------------------------------------------------
There is one problem with my patch:
@Override
public Iterator<? extends T> iterator(int first, int count) {
ArrayList<T> result = new ArrayList<T>(count);
Here we shouldn't preallocate space in ArrayList, since in some views (w/o paginators) the count is equals to Integer.MAX_VALUE.
So the fixed code is:
@Override
public Iterator<? extends T> iterator(int first, int count) {
ArrayList<T> result = new ArrayList<T>();
was (Author: mihasik):
There is one problem with my patch:
@Override
public Iterator<? extends T> iterator(int first, int count) {
ArrayList<T> result = new ArrayList<T>(count);
Here we shouldn't preallocate space in ArrayList, since in some views (w/o paginators) the count is equals to Integer.MAX_VALUE.
So the fixed code is:
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
> Attachments: AbstractDataProviderDecorator.java, OptimizedDataProviderDecorator.java
>
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-2618) Don't call IDataProvider.size
method when IDataProvider.iterator(first, count) returned the number of
elements less than "count"
Posted by "Michael Mikhulya (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12801771#action_12801771 ]
Michael Mikhulya commented on WICKET-2618:
------------------------------------------
There is one problem with my patch:
@Override
public Iterator<? extends T> iterator(int first, int count) {
ArrayList<T> result = new ArrayList<T>(count);
Here we shouldn't preallocate space in ArrayList, since in some views (w/o paginators) the count is equals to Integer.MAX_VALUE.
So the fixed code is:
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
> Attachments: AbstractDataProviderDecorator.java, OptimizedDataProviderDecorator.java
>
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (WICKET-2618) Don't call IDataProvider.size
method when IDataProvider.iterator(first, count) returned the number of
elements less than "count"
Posted by "Michael Mikhulya (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793972#action_12793972 ]
Michael Mikhulya commented on WICKET-2618:
------------------------------------------
no changes in API required
Size will be calculated precisely from result of "iterator" when it is possible. In case when it is not possible then decorator will forward to size() of delegate object.
I believe changes shouldn't be complex, will check it on monday.
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (WICKET-2618) Don't call IDataProvider.size method
when IDataProvider.iterator(first, count) returned the number of elements
less than "count"
Posted by "Michael Mikhulya (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WICKET-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Mikhulya updated WICKET-2618:
-------------------------------------
Attachment: OptimizedDataProviderDecorator.java
AbstractDataProviderDecorator.java
There is a class from my application which already provide optimization benefits to my application:
public abstract class CompoundDataView<T> extends DataView<T> {
protected CompoundDataView(String id, IDataProvider<T> dataProvider) {
super(id, new OptimizedDataProviderDecorator<T>(dataProvider));
}
protected CompoundDataView(String id, IDataProvider<T> dataProvider, int itemsPerPage) {
super(id, new OptimizedDataProviderDecorator<T>(dataProvider), itemsPerPage);
}
@Override
protected Item<T> newItem(String id, int index, IModel<T> model) {
return super.newItem(id, index, new CompoundPropertyModel<T>(model));
}
@Override
protected int getViewSize() {
return internalGetRowsPerPage();
}
}
The important parts are usage of OptimizedDataProviderDecorator and overriding of getViewSize.
This optimization is safe with current release of wicket (getViewSize is called only from getItemModels which I can't override because of private class CappedIteratorAdapter).
Of course it is safer to use internalGetRowsPerPage instead of getViewSize in AbstractPageableView.getItemModels.
Notice, as soon as this optimization is integrated in wicket we can remove "cachedItemCount" related methods from AbstractPageableView.
This optimization already saves me "count" sql in case when number of found elements is less than number of rows per page.
Notice I provided a fix only for AbstractPageableView. (In my application I don't use other subclasses of IPageable so it will take me much more time to implement and test it).
Probably this optimization can be used for case when we ask for last page too. Now it does't work for PagingNavigationLink, since here we call pageable.setCurrentPage in onClick and IDataProvider.size is called prior to "iterator" method.
> Don't call IDataProvider.size method when IDataProvider.iterator(first, count) returned the number of elements less than "count"
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: WICKET-2618
> URL: https://issues.apache.org/jira/browse/WICKET-2618
> Project: Wicket
> Issue Type: Improvement
> Components: wicket
> Affects Versions: 1.4.4
> Reporter: Michael Mikhulya
> Attachments: AbstractDataProviderDecorator.java, OptimizedDataProviderDecorator.java
>
>
> IDataProvider has two mehtods:
> Iterator<? extends T> iterator(int first, int count);
> int size()
> which in many implementations do access to database.
> Normally iterator do SQL query with LIMIT and OFFSET, and size do SQL COUNT query.
> SQL COUNT is O(N) for most transactional databases (e.g. for postgresql) and so it is rather heavy.
> In some cases it is possible to avoid call to IDataProvider.size method.
> For example when "iterator" returned the number of elements less than acquired "count".
> This case appears in 90% in my application (whenever search is used normally number of found elements is less than number of elements on page).
> Also pgFouine shows me these queries as the most slow one. So I suggest to implement this optimization.
> First I though that appropriate place to implement it is AbstractPageableView class, but unfortunatelly not all IPageable classes extends this abstract class (for example DataTable doesn't do so).
> So I suggest to implement Decorator of IDataProvider and reuse it in DataViewBase, DataTable, ...
> If nobody see any problems with this suggestion I would like to implement it on my own.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.