[Monotone-devel] Updated Issue 120 - list changed much slower than status (monotone)

[code]

Hello,

The following issue has been updated:

120 - list changed much slower than status
Project: monotone
Status: Fixed
Reported by: joe 23
URL: https://code.monotone.ca/p/monotone/issues/120/
Labels:
 Type:Defect
 Priority:Medium
 Milestone:1.0

Comments (last first):

# By Richard Hopkins, Feb 24, 2011:

1ce52abe1d12b891c5c5783921287f41b5915b30 passes all the tests and now needs 
reviewing.

 Status: Fixed

# By Thomas Keller, Feb 18, 2011:

@Richard H.: Any news on this one?

# By Richard Hopkins, Feb  6, 2011:

Thanks for reviewing - I'm currently on Windows and cannot build it at the 
moment but I'll look at it again soon.

"externalize the code that iterates the edge_map" - do you mean to put the for 
loop in a new function so it can be used by "mtn status" and "mtn ls changed" - 
because they are now practically identical ?

I'm also not sure what you mean by namespaced enum ? I guess you mean 

enum output_type { added, attr, dropped, patched, renamed };

Does that mean the method would then need a set<output_type> and only if the 
set contains "added" should the added files be output ?

Regarding the reason for the slowness, I couldn't actually reproduce it myself 
- but the original issue mentioned a 750MB and I don't have one that big.

However, I did compare both versions and the only difference was inside the 
edge_map for loop. The start of both functions performed the same things but 
just in a different order.

The inside of the edge_map seemed to be doing basically the same as well - the 
original called select_nodes_modified_by_cset - which loops around each type 
just like the new implementation does.

The only difference I can see is that select_nodes_modified_by_cset does extra 
loops at the end to copy what it has found into the output. 
select_nodes_modified_by_cset also has more memory overhead as it's keeping 
more track of more things in memory.

# By Thomas Keller, Feb  5, 2011:

I had a look at the changes in this branch and you're heading the right 
direction. Please go the extra mile and externalize the code that iterates the 
edge_map, but instead of using booleans, use some local, namespaced enum.

Secondly, your branch is missing a short note in NEWS that this bug is fixed by 
the change. Its a good idea to do that in the branch already, so its not 
forgotten later on and can be reviewed as well. 

Finally, I'm not sure I haven't asked that before, but could you find out the 
reason (or reproduce at least) why the old implementation was so slow?

 Status: Started

# By Richard Hopkins, Jan 25, 2011:

This has passed preliminary testing, and is now ready for extensive testing.

See fc13d22f21a87c3a0f37d87a7fb8bbf667b65639 for more details.

 Status: Fixed

# By Richard Hopkins, Jan 25, 2011:



 Labels: Milestone:1.0
 Status: Accepted

# By joe 23, Dec 26, 2010:

It seems like 'list changed' is the same as 'status' but with a different 
output format. However, 'list changed' takes much longer, when a small portion 
of the total workspace is specified. With a decent-sized project, the delay of 
'list changed' is mildly annoying.

My guess is that 'status' only checks the portion of the workspace specified, 
but 'list changed' checks the whole workspace and then filters the result?

Of course if there's some good reason why it must be slower, please close the 
ticket.

Thanks!

Steps to reproduce the problem:
-------------------------------

1. time mtn list changed path/to/some/dir
2. time mtn status path/to/some/dir
3. repeat 1) and 2) now that the OS has cached things

Expected result:
----------------
Both take about the same CPU and elapsed time

Actual results:
---------------
list changed takes about 4x as long as status, and is noticeably slower to 
complete.


Output of `mtn version --full`:
-------------------------------
0.99.1



--
Issue: https://code.monotone.ca/p/monotone/issues/120/