Caching foundation for a dependencyGraph which maps dependency relations.

[ashprice]

This is an initial PR which lays the groundwork for one that will shortly follow. It introduces lazy caching of pending/completed tasks, a UUID index for pending tasks, the construction of a dependency map, and adds in-place mutation of pending tasks. This roughly halves the instruction count for many reports (though the speed gain from this PR is minor to the one that I will make if it is accepted, about 1.3x for many reports. I gave up waiting for the performance tests in ./performance/ to complete, and instead constructed a smaller, 300 task test database.)

Changes

• pending_tasks() and completed_tasks() return const std::vector<Task>& instead of copying. all_tasks() is deliberately not cached, as it can be very large and is only read once per invocation.
• an index of pending task UUIDs, _pending_index is lazily built with pending_tasks(). get() and modify() use this instead of scanning the vectors.
• A DependencyGraph hashmap is introduced and built from the pending cache. (A temporary map is created for all_tasks(), in the event that dependency_scan() must refer to things in all_tasks.) This is not yet consumed by Task in this PR - that will be the PR to follow.
• modify() makes updates in-place within the vector by leveraging the _pending_tasks index. This cache is invalidated in any case where there may be a change of state between the Rust replica and the cached index.
• some minor changes relating to memory management for pending_tasks and completed_tasks.
• use the new cache in siblings()/children(), as well as get_ref() to avoid copying. This also fixes a non-user-visible bug where children() would return different is_blocked()/is_blocking() flags to what you would expect.

Endnotes

This PR doesn't necessarily bring much by itself, but it's needed to lay the groundwork for DependencyGraph to be used by getDependencyTasks() and getBlockedTasks(). Adding in in-place modifications is a somewhat opportunistic addition for the PR, as it's not strictly necessary for core functionality, but it does improve modification performance when running big batch modifications.

All tests pass and I am pretty sure this doesn't break anything, but there's a small chance bugs have slipped by me.

This would be my first major contribution to a C++ codebase, and I am not a professional software engineer, so feedback on pretty much everything here is accepted gratefully.

[ashprice]

Work on further PRs is ongoing on other branches of that repo. If you'd prefer I submit the whole series as one and look at it at once, I will make this a draft PR for now. I didn't want to overload you with hundreds of lines of changed code at once. :)

Edit: Also, I'd be happy to try looking at the rust side given I much prefer rust to C++, after this. I thought it better to see what could be done on the TW side, and then think about that.

If the TC FFI lands, it'll likely change what should be focused on, as well.

I don't mind if most of the code in these PRs ultimately gets thrown out after the fact because of improvements in interoperability, this work was mostly born out of frustration with v3's performance in my own daily usage.

On my own local build, I've reached the point where improvements may cut instruction count but have no or negative impact on performance for both rc.gc=1 and rc.gc=0 in release builds. Maybe there is something I cannot spot, of course, but I think any remaining gains will be found in TC and not in TW.

Also, 240ms (70-80ms with gc=0) on my own database for a ready report is pretty fast - much faster than TW v2. Given that was a significant pain point for many users after the migration, and potentially made some of them switch to other tools, it would make me glad to address it.

Modifications are still a bit slower than v2 I think, but still, pretty damn fast. Most of that gain is from handing off the work to rust, but modifying the in-memory cache with the same information. Given the cache invalidations I put in place, I think this is safe and unlikely to lead to race conditions.

Also: I wouldn't have been able to do any of this without the excellent test suite I think. I would have messed something up. And I doubt I could have written tests that are as comprehensive. So a big thank you goes to previous contributors for that!

[ashprice]

Sorry about the unnecessary bump. Clippy test was failing because of it. Used to merging changes from upstream into personal repos. I looked up git best practices after the fact and realized I shouldn't merge/rebase unnecessarily if there aren't conflicts.

[djmitche]

This looks good! These are minor points so feel free to say "no". I'll merge once I've heard back from you.

[djmitche]

Given that task generally runs from start to finish (it's not a daemon), I'm not sure this is necessary. If we've loaded all tasks once, we've allocated the memory, and are unlikely to re-use that memory usefully before the process exits.

I don't necessarily think it's worth adding a caching implementation, but this comment might push future developers in the wrong direction.

[ashprice]

You're right! Do you think I should strip the comment out completely, or remove the part after the comma?

[ashprice]

There's another comment I need to amend about the +BLOCKING bug as what I wrote is not strictly accurate so I will wait for a reply and make a final comments commit. I'm going to make a review comment there because, honestly, I'm not sure about the change.

[ashprice]

Sorry @djmitche I accidentally overwrote the previous comment here. I agree with you, if it isn't found, we should invalidate the cache.

[djmitche]

Awesome, thank you so much!