fix performance issue when exporting many groups

[ltalirz]

When providing groups to verdi export, it was looping over all groups
and using the Group.nodes iterator to retrieve the nodes contained in
the groups. This results (at least) in one query per group, and is
therefore every inefficient for large numbers of groups. The new
implementation replaces this by a single query.

[lekah]

Is there any benchmark where you compare times to get the result? It should depend on what group.nodes is exactly doing, but in the end I guess the performance bottleneck is loading the nodes, not the query. Just out of interest (since the title is "fix performance issue")

[lekah]

Could you use group_qb.iterall() here? It might lead to further improvement because the results would be fetched in batches in such case and not all together as is done now. If the subsequent code also takes time this could speed up the time-to-solution.

[ltalirz]

Is there any benchmark where you compare times to get the result?

Here come some actual numbers.

Test set: export of 67k groups containing 5 nodes each.
Old implementation: 204s
New implementation with qb.all(): 64s
New implementation with qb.iterall(): 59s

It should depend on what group.nodes is exactly doing, but in the end I guess the performance bottleneck is loading the nodes, not the query.

My guess was that the bottleneck are the N queries you make for N groups and I think the benchmark results back this up.

However, as you say and as I point out in the comment, there might still be significant speedup from avoiding to construct the ORM objects altogether, as well as memory savings.

@sphuber: Do you happen to know whether there is an easy check to replace these checks

aiida-core/aiida/tools/importexport/dbexport/__init__.py

Lines 232 to 235 in 9172c60

	if issubclass(entry.__class__, orm.Data):
	given_data_entry_ids.add(entry.pk)
	elif issubclass(entry.__class__, orm.ProcessNode):
	given_calculation_entry_ids.add(entry.pk)

by checks on the node_type?

[sphuber]

Yes, you can select those sets of nodes directly on the node_type.
For Data nodes:

SELECT * FROM db_dbnode WHERE node_type LIKE = 'data.%';

and for ProcessNode nodes, i.e. all other

SELECT * FROM db_dbnode WHERE node_type LIKE = 'process.%';

In those particular query builder definitions you can simply make two builders querying on the specific node class

builder_data = orm.QueryBuilder().append(
    orm.Group, filters={'id': {'in': given_group_entry_ids}}, tag='groups').append(
    orm.Data, with_group='groups', project='*')
builder_process = orm.QueryBuilder().append(
    orm.Group, filters={'id': {'in': given_group_entry_ids}}, tag='groups').append(
    orm.ProcessNode, with_group='groups', project='*')

[ltalirz]

Thanks @sphuber - the latest implementation now takes 11s on the test set (down from 204s on develop).

[ltalirz]

Without this I get an error that zip is a module and not callable ( we have a module called zip in the same folder).
If you want me to rename the module instead, let me know.

[ltalirz]

@sphuber This is ready for review

[sphuber]

Good stuff, thanks a lot @ltalirz !

[ltalirz]

Do you want me to write the commit message?
Or are you waiting for another review?

[sphuber]

Do you want me to write the commit message?
Or are you waiting for another review?

No, I was just leaving the honors to yourself, commit message looks great :)

[CasperWA]

For the future: This should be changed in #3242