Added SamplingResult cdef class to return cupy "views" for PLC sampling algos instead of copying result data

[rlratzel]

Summary

• (partially) closes Add support for server GPU- client GPU data xfer GaaS#23
• Added SamplingResult cython extension class that corresponds to the C API's cugraph_sample_result_t type, which removes a copy operation in sampling algos.
• Added new testing APIs and unit tests to validate correctness and that memory is cleaned up as expected when all refs to results are removed (see test_neighborhood_sampling.py::test_sample_result)
• Updated build.sh to use ninja by default.

Background

As part of a larger effort to provide GPU-GPU data transfer in cugraph_service to improve performance, the need to minimize redundant data copy operations was also something to address.

Prior to this change, PLC algos would copy their results from the C API device data structures into one or more cupy ndarrays, which allowed the algo to then delete the result data owned by the C API and leave the lifecycle management of the results returned to python to the python interpreter/GC. The copy operation not only slowed down sampling algo calls (slightly) but also caused spikes in GPU memory usage where two copies of results existed in GPU memory for a short time immediately after the copy but before the original was deleted.

This PR adds a cython extension class that corresponds to the C API's cugraph_sample_result_t type, which is used to pass ownership of the sampling results to python. The extension class calls cugraph_sample_result_free() when all references to the result data in python are removed and the garbage collector runs. The owning SamplingResult object allows PLC sampling algos to instead return cupy ndarray "views" for results instead of copying them to new cupy ndarrays.

New test APIs to create sampling result objects in C and PLC were added and used in the new tests to verify that the ownership of the result data was properly transferred to Python, and that the lifecycle management of results was done properly when clients keep references to data (eg. pass results to other parts of code for use long after the algo completes).

A benchmark was also added to verify the (minor) speedup. Runs 0001-2 were before the change, runs 0003-5 (red box) were after:

[codecov-commenter]

Codecov Report

Base: 60.04% // Head: 57.76% // Decreases project coverage by -2.27% ⚠️

Coverage data is based on head (7e80e59) compared to base (3eb2b40).
Patch has no changes to coverable lines.

Additional details and impacted files

@@               Coverage Diff                @@
##           branch-22.10    #2684      +/-   ##
================================================
- Coverage         60.04%   57.76%   -2.28%     
================================================
  Files               111      111              
  Lines              6184     6069     -115     
================================================
- Hits               3713     3506     -207     
- Misses             2471     2563      +92     

Impacted Files	Coverage Δ
python/cugraph/cugraph/structure/property_graph.py	65.02% <0.00%> (-31.87%)	⬇️
...n/pylibcugraph/pylibcugraph/utilities/api_tools.py	62.50% <0.00%> (-15.63%)	⬇️
python/cugraph/cugraph/link_analysis/hits.py	78.26% <0.00%> (-5.62%)	⬇️
...graph/cugraph/centrality/eigenvector_centrality.py	90.47% <0.00%> (-3.47%)	⬇️
...thon/cugraph/cugraph/centrality/katz_centrality.py	85.71% <0.00%> (-3.42%)	⬇️
python/cugraph/cugraph/sampling/node2vec.py	78.78% <0.00%> (-3.04%)	⬇️
python/cugraph/cugraph/structure/graph_classes.py	78.46% <0.00%> (-1.54%)	⬇️
python/cugraph/cugraph/traversal/sssp.py	92.79% <0.00%> (-0.65%)	⬇️
python/cugraph/cugraph/structure/number_map.py	67.49% <0.00%> (-0.36%)	⬇️
...ugraph/cugraph/dask/structure/mg_property_graph.py	14.79% <0.00%> (-0.31%)	⬇️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[rlratzel]

I believe the CI test failures are related to the changes in this PR, so I'm looking into those.

[ChuckHastings]

I would be inclined to do this a bit differently. The values new_device_srcs, new_device_dsts, new_device_wgts and new_device_counts that you're creating here are a potential for a memory leak, since they are not in RAII objects.

You could:

1. Do type dispatching and create these as rmm::device_uvector<appropriate_type> and then convert to the return value. This would mimic the pattern of the algorithm implementation.
2. Create these as std::unique_ptr values and release them when you convert to the return value
3. Create the return value as a std::unique_ptr (releasing upon return) with the new calls for these operators inside and then just use the values from the return object. We do know the sizes a priori

[rlratzel]

Instead of this suggestion, I changed the implementation to use other cugraph C API calls (as suggested below), which included using cugraph_type_erased_device_array_create_from_view() from #2712 . Let me know if you'd prefer I use the rmm:device_uvector/type dispatching approach instead.

[BradReesWork]

is there, or should there, be a macro for if (error_code != CUGRAPH_SUCCESS) return error_code;?

[galipremsagar]

rerun tests

[rlratzel]

@gpucibot merge