History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: HBX-46
Type: Improvement Improvement
Status: Open Open
Priority: Major Major
Assignee: Unassigned
Reporter: Max Rydahl Andersen
Votes: 1
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Hibernate Tools

allow queryresults to be shown lazy

Created: 08/Dec/04 07:00 AM   Updated: 27/Oct/06 08:34 AM
Component/s: consoleconfiguration
Affects Version/s: None
Fix Version/s: 3.2LATER

Time Tracking:
Not Specified

File Attachments: 1. Text File hibernateConsole_queryPageViewerthreadandResouces.patch (3 kb)
2. Text File JIRAHB46.org.hibernate.eclipse.console.patch (15 kb)
3. Text File JIRAHB46.org.hibernate.eclipse.console.patch (4 kb)
4. Text File JIRAHB46.org.hibernate.eclipse.patch (12 kb)
5. Text File JIRAHB46.org.hibernate.eclipse.patch (8 kb)

Image Attachments:

1. StrategySelectionToggles.jpg
(40 kb)
Issue Links:
Relates
 

Community Help Wanted: Yes, please


 Description  « Hide
Currently the QueryPageViewer converts the list returned from Hibernate to an array immediatly - making it not very scalable for larger results

 All   Comments   Work Log   Change History   FishEye      Sort Order: Ascending order - Click to sort in descending order
David Martinez - 16/Mar/05 09:27 AM
Hello, this patch will fix that problem. It also fixes a problem where the queryPage items were not being released, causing eclipse to lock up after a couple of queries.

So not the behavior is:

1) Show "Query Running.."
2) Fire off a thread to go fetch the results
3) When the results are obtained, update the table.

Now, if a query locks up because of the pool size limitations, just close one of the other threads.

Have fun!


Max Rydahl Andersen - 16/Mar/05 09:50 AM
Thank you for the patch.

Unfortunately i doesnt solves the problem, it only hides it.

The result is still converted into a full array.
The proper solution would be to allow users to select if they want to use list, iterate or scroll + make the underlying tablemodel virtual to avoid excessive memory usage.

David Martinez - 16/Mar/05 10:10 AM
You're right about the memory usage.

I see that as a limitation of the ContentProvider implementation more than the queryResults though. It expects you to retrieve an array instead of working like the swing tablemodel. I am sure I can do something further on this to get a max fetch size configuration item so it will stop there. I am unfortunately still a hibernate beginner :-) But I'm sure I can do something about it.

Max Rydahl Andersen - 16/Mar/05 10:17 AM
yes swt isnt very scalable by default BUT in eclipse 3.1 they have added VIRTUAL support to trees and table. googling for it should give some hints.

David Martinez - 16/Mar/05 10:57 AM
Thanks, I had heard about this, but I haven't coded any virtual tables yet (product still in 3.0 for me) so I didn't know where to start.

I'll take a look at adding support for this. I thought I was going to have to replace the TableViewer but apparently they also added support on the viewers - and I can see that support on my M4 source. https://bugs.eclipse.org/bugs/show_bug.cgi?id=72358

Now I have to go fetch the sample code.

Thanks again,

Max Rydahl Andersen - 17/Mar/05 08:21 AM
downgrading priority as it is not blocking our release.

still appreciate any patches you can come up with for this!

David Martinez - 09/Nov/05 06:19 PM
I was taking a look at it and started on the virtual table. However the problem is that the only possible way to keep the memory down is with scroll():

list() of course will retrieve the world.
iterate() will iterate, but the calling thread for the notification will still have to keep all the iterated items in memory.
scroll() may work if we keep making the table poll for data

In order to cover all three uniformly (and not drive the UI implementer crazy having to use three different strategies on the IStructuredContentProvider), I thought about using an event mechanism, something like:

/**
* Starts the query in the background.
*/
public void startQueryInBackground(final int queryType, final DataFetchedListener listener) {
setupParameters(query, queryParameters);
new Thread() {
public void run() {

switch(queryType) {
case DataFetchedEvent.QUERY_EXEC_TYPE_ITERATE: {
doIterateQuery(query, listener);
break;
}
case DataFetchedEvent.QUERY_EXEC_TYPE_LIST: {
doListQuery(query, listener);
break;
}
case DataFetchedEvent.QUERY_EXEC_TYPE_SCROLL: {
doScrollQuery(query, listener);
break;
}
}

}
}.start();
}

private void doIterateQuery(Query query, DataFetchedListener listener) {
Iterator iter = query.iterate();
while ( iter.hasNext() ) {
Object nextObject = iter.next();
DataFetchedEvent evt = new DataFetchedEvent(DataFetchedEvent.QUERY_EXEC_TYPE_ITERATE, this, nextObject);
try {
if ( ! listener.dataWasFetched(evt) ) return;
} catch (Throwable thr) {
thr.printStackTrace();
// TODO Log error in event. Happened in a listener so troll along.
}
}
}

// private void do.....() and so on

Then queryPageViewer could be something like this (a lot more cleaned up):

class ContentProviderImpl implements IStructuredContentProvider, DataFetchedListener {
Vector readData = new Vector();

public Object[] getElements(Object inputElement) {
return readData.toArray();
}

public void inputChanged(Viewer viewer, Object oldInput, Object newInput) {
// TODO UI for override.
int queryType = DataFetchedEvent.QUERY_EXEC_TYPE_ITERATE;
queryPage.startQueryInBackground(queryType, this);
}

public boolean dataWasFetched(DataFetchedEvent evt) {
// TODO: 1. Check the type of fetch. 2. Add to the current contents here. 3. Fire inputChanged
if ( DataFetchedEvent.QUERY_EXEC_TYPE_ITERATE == evt.getQueryExecType() ) {

if(! queryPage.getExceptions().isEmpty() ) {
readData.addAll(queryPage.getExceptions());
//HibernateConsolePlugin.getDefault().logErrorMessage("Exception while executing HQL Query", throwables);
}

return true;
}

}

But as I was saying there is no "viewport" idea unless we can go back and forth, and the APIs are too different to do so.

Do you prefer this type of approach, or more of a "IStructuredContentProvider polls" mechanism?


Max Rydahl Andersen - 10/Nov/05 04:20 AM
hmm - if we could just have swing ;)

In any case, I don't think it will be good to have a worker thread that could hold on to the session indefinitly - and relying on scroll() for "scrolling"
is not good since not all db's support scroll() in both directions etc.

My main issue with the memory consumption is not so much the session, but the fact that SWT table model contains a copy of the data redundantly (unless you do virtual table tricks)

Thinking about it I would say that a good solution will contain the following elements:

The user enters a query.
The user can enter values for maxRows, possibly a firstResult and a "paging-size"

When we execute the query it will be done in a Job so the UI does not freeze and the result will be delivered in a "big chunk" (having it come in piece by piece is a nice thing to have, but we cannot use it for much since we cant use the session in multiple threads)

When the user browses the data we can fetch the remaining data with the above information.

Optionally the user can choose wether he wants us to use list, iterate or scroll to fetch the result - but that will just be used
to effect the type of sql that will be executed, not the amount of data.

Comments?




David Martinez - 10/Nov/05 09:37 AM
Yes, for table, nothing beats AbstractTabelModel :-) Agreed about the job too.

About the virtual table tricks, we could forgo the IStructuredContentProvider/ILabelProvider and use the SWT directly using the setData event so SWT doesn't keep a copy of the data - (or does it anyway? I forget).

The maxRows, firstResult and pageSize idea sounds great. However for that we may need a property page to set defaults (maybe on Window->Preferences). A sensible default may be 1000 maxRows, 1 firstResult and no pageSize defined.

This would be in addition to the control already in the query view page (probably in a button, the same way the "Filters..." works on the Problems view.

David Martinez - 10/Nov/05 10:10 AM
So the list/iterate/scroll is to force the query generation to behave as it would in the different cases you would be using this hql. Cool. Is there a sql log view anywhere currently?

David Martinez - 10/Nov/05 10:32 AM
One other thing: I noticed that none of the org.hibernate.console classes use eclipse-specific stuff. I'd like to provide proper progress monitoring to the querying process, but I'd have to use IProgressMonitor.

If this is a not a hard-fast rule I can just have AbstractQuery have a get/set on IProgressMonitor.

If it is, (i.e. you use this in a Swing UI as well), I can create a separate monitoring interface and have the job implement that interface instead (by delegating to the IProgressMonitor), with AbstractQuery having a get/set on something similar called QueryProgressMonitor.

Any recommendations on this?

Max Rydahl Andersen - 10/Nov/05 11:26 AM
in transit right now so cant answer in detail, will do later.

but we have a org.hibernate.cfg.reveng.ProgressListener which you are very welcome to use/extend for progressmonitoring.

David Martinez - 10/Nov/05 12:45 PM
This seems to do it using an eclipse job. I still need to put the UI for the options.

David Martinez - 10/Nov/05 04:13 PM
This illustrates what the strategy selection toggles will look like. I also copied the progress job name on the bottom right to give an idea of the results.

David Martinez - 10/Nov/05 04:15 PM
Second patch (includes the first one). This one also has the actions with the strategy selection as illustrated in the screenshot.

Max Rydahl Andersen - 11/Nov/05 07:58 AM
I tried to appy your patch and it seems to work - good job!

I'll have too look into it for some more detail and we need to expose the firstResult, maxResult and strategy better in the ui.
Maybe just add a toolbar to the HQL Editor with a Run button, a cfg dropdown, a execution dropdown (.list(), .iterate(), .scroll(), .executeUpdate()) and fields for firstResult and maxResult (where -1 will mean not specified)

p.s. notice the executeUpdate() here - it is just something that would fit in nicely.

Let me know if it is something you would like to do.

Regarding if there is a sql log view, then no there is not, but should be fairly easy to add since we now have Interceptor.prepareSQL() which we can use to track it.

Max Rydahl Andersen - 11/Nov/05 08:09 AM
btw. would be good with one big patchfile (or at least name if with a timestamp) since im now getting many swt exceptions in the worker threads and i think maybe its because eclipse didn't apply all of the patch and my manual copying might have been imprecise.

David Martinez - 14/Nov/05 09:59 AM
Thanks!

I agree that the toolbar directly on the editor would be better, though the design you're describing conflicts with the current "Run" button over in the toolbar (the one where the attached menu is the configuration to use). It is however probably more intuitive to use list/iterate/scroll/executeUpdate as the dropdown.

Sorry about the Thread access swt exceptions. I'm pretty sure I fixed those before submitting the files again (it was a problem with the way I was calling the deferred update - out of old habits I was using awt's EventQueue.invokeLater instead of Display.getDefault().asyncExec) - I thought I'd be able to remove the old patches since I was the one that submitted them but JIRA won't let me. Oh well, the files are currently 1 and 3 on that list (looks like the sort is reverse alpha+reverse timestamp). Next time I'll submit a single file patch with the timestamp. Try it like that and let me know if you still get swt exceptions.

Max Rydahl Andersen - 14/Nov/05 11:56 PM
i were planning on removing the current "Run" button functionallity with this new design. (the drop down button would still be possible, but I think it would be better to move the run button closer to the editor together with the additonal buttons/dropdowns)

I'll try and apply your patch when i get the chance again.

Max Rydahl Andersen - 24/Apr/06 05:53 AM
i tried to apply it, but there were issues with it (I just can't remember what :(