[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [p2-dev] Review wanted. Simplifying the IQueryResult/IQuery API

I have not reviewed the patch but I'm not a big fan of the removal of size and toCollection.
They end up being particularly useful in some places, especially to convert from one type to the other, or eventually create collections with the right size up front (which saves a lot of growth when you work on very large collections).

I agree that there are cases where all the IUs can't be held in memory, however there are cases in today's code (engine and planner) where I don't see how not having all the IUs in memory is possible, and where the presence of such methods is necessary.
In the end I think I could probably leave without the toCollection, but loosing size would likely cause performance issues.

PaScaL


Inactive hide details for Thomas Hallgren ---17/12/2009 05:31:38 PM---Hi,Thomas Hallgren ---17/12/2009 05:31:38 PM---Hi,


From:

Thomas Hallgren <thomas@xxxxxxx>

To:

P2 developer discussions <p2-dev@xxxxxxxxxxx>

Date:

17/12/2009 05:31 PM

Subject:

[p2-dev] Review wanted. Simplifying the IQueryResult/IQuery API




Hi,
I've done some work to simplify the IQueryResult/IQuery API further.
Once Ian's patch came in yesterday, it was possible for me to move
forward on two things that I wanted to do for some time.

1. Establish a clear separation of concern between query processing and
the processing of a query result.
This item was very easy to do now. The work entailed three things:

- Removal of the addAll() method from the IQueryResult
- Make IQuery.perform() return an IQueryResult instead of a Collector
- Remove the Collector argument from the IQuery.perform()

Now the IQuery looks like something that produces a read-only result.
This is very much in line with how the QL is designed and will make the
integration that I'd like to do as the next step very easy. It will also
vouch for some furhter performance improvements since we can write very
streamlined IQueryResults for different purposes.

2. Avoid use of collections as long as possible.
A query can produce a huge result. Huge results take up a lot of memory.
Consequently, a good 'query' design should not encourage the use of
collections unless they are absolutely necessary. I did the following to
address this:

- Removed the 'size()' from IQueryResult
- Removed the 'toCollection()' methods from the IQueryResult

The toCollection() would always create a copy of the Collector
collection. In many cases, this collection was obtained only to pass as
an argument to the constructor of another collection. In some cases, the
collection was only obtain in order to get the size. Now, when neither
method is there, it's less intuitive to use a collection and more
intuitive to use an iterator. This is a good thing since the query
result may be very large. In case of QL, it may not contain a collection
in the first place.

I just submitted a new attachment to
https://bugs.eclipse.org/bugs/show_bug.cgi?id=298023 because the other
one went stale almost immediately. Not surprising since it touches a lot
of files. But it does show that a review is somewhat urgent. So please,
if you could take a look and give me a go ahead, or a reason not to, I'd
appreciate it.

Thanks,
Thomas Hallgren

_______________________________________________
p2-dev mailing list
p2-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/p2-dev


GIF image

GIF image