Persist more in MGPropertyGraph

[eriknw]

This should fix the quadratic scaling we're seeing when adding new data.

CC @VibhuJawa. I'm still trying to improve the merges for MG to be like #2796, but I'm encountering issues.

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.12@856e3ba). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@               Coverage Diff               @@
##             branch-22.12    #2805   +/-   ##
===============================================
  Coverage                ?   60.48%           
===============================================
  Files                   ?      111           
  Lines                   ?     6511           
  Branches                ?        0           
===============================================
  Hits                    ?     3938           
  Misses                  ?     2573           
  Partials                ?        0           

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.

[VibhuJawa]

I think it looks good . We should just add a small test to ensure they are actually persisted. Looks good other-wise.

[alexbarghi-nv]

This looks fine to me, I just want to verify that this actually increases performance. Have you run some sort of benchmark before/after persistence?

[eriknw]

Yeah, I found this PR to be much faster when experimenting with the MAG240 dataset.

To satisfy @VibhuJawa and @alexbarghi-nv comments, I will add a benchmark as a test and will share performance results here.

[eriknw]

Benchmark "test" added.

Results before this PR:

------------------------------------------------------------- benchmark: 3 tests ------------------------------------------------------------
Name (time in s, mem in bytes)        Mean                GPU mem            GPU Leaked mem            Rounds            GPU Rounds
---------------------------------------------------------------------------------------------------------------------------------------------
bench_add_edges_cyber[1]            1.7739 (1.0)      273,342,160 (1.0)                 232 (1.0)           1           1
bench_add_edges_cyber[3]            5.3522 (3.02)     273,342,568 (1.00)                632 (2.72)          1           1
bench_add_edges_cyber[10]          30.4148 (17.15)    273,343,760 (1.00)              1,840 (7.93)          1           1
bench_add_edges_cyber[30]   DNF
---------------------------------------------------------------------------------------------------------------------------------------------

Results with this PR:

------------------------------------------------------------- benchmark: 4 tests ------------------------------------------------------------
Name (time in s, mem in bytes)        Mean                GPU mem            GPU Leaked mem            Rounds            GPU Rounds
---------------------------------------------------------------------------------------------------------------------------------------------
bench_add_edges_cyber[1]            1.5217 (1.0)      273,342,296 (1.0)                 360 (1.0)           1           1
bench_add_edges_cyber[3]            2.7838 (1.83)     273,342,432 (1.00)                496 (1.38)          1           1
bench_add_edges_cyber[10]           7.6317 (5.02)     273,343,480 (1.00)              1,544 (4.29)          1           1
bench_add_edges_cyber[30]          18.6916 (12.28)    273,344,064 (1.00)              2,144 (5.96)          1           1
---------------------------------------------------------------------------------------------------------------------------------------------

Results for SG PropertyGraph:

------------------------------------------------------------------ benchmark: 14 tests ------------------------------------------------------------------
Name (time in ns, mem in bytes)                   Mean                GPU mem            GPU Leaked mem            Rounds            GPU Rounds
---------------------------------------------------------------------------------------------------------------------------------------------------------
bench_add_edges_cyber[1]              136,008,216.0681 (>1000.0)  494,686,464 (inf)                   0 (1.0)           1           1
bench_add_edges_cyber[3]              235,328,749.7535 (>1000.0)  468,917,688 (inf)                   0 (1.0)           1           1
bench_add_edges_cyber[10]             576,618,617.4005 (>1000.0)  454,553,424 (inf)                   0 (1.0)           1           1
bench_add_edges_cyber[30]           1,493,484,066.7993 (>1000.0)  450,488,352 (inf)                   0 (1.0)           1           1

Merging in the style of #2796 should increase performance of both SG and MG significantly, but MG is waiting on a fix for cudf.

[VibhuJawa]

LGTM

[rlratzel]

LGTM, thanks for adding the benchmarks too.

Re: benchmarks - are the GPU mem leak numbers actual leaks, or just something related to how we're testing?

[alexbarghi-nv]

This is good to go with the benchmarks, thanks @eriknw

[alexbarghi-nv]

👍

[rlratzel]

@gpucibot merge