Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OPTIMIZATION] Explicitly make xidmap shards as nil #4738

Merged
merged 2 commits into from
Feb 6, 2020

Conversation

ashish-goswami
Copy link
Contributor

@ashish-goswami ashish-goswami commented Feb 6, 2020

In bulk/live loader, we use xidmap to keep mapping of external ids to UUIDs. In case of bulk
loader, this is required only in map phase. Hence at the completion of map phase we make
xidmap instance used in bulk loader as nil. Ideally this should be good for xidmap GC and
release of underlying memory. But this is not happening and xidmap still shows up in memory
profiles even during reduce phase.

This is fixed by making xidmap.shards = nil in xidmap.Flush() method.

Bulk loader memory profile on 21M dataset during reduce phase:
Profile has been taken after reduce phase is completed 50%.
Master

(pprof) top20
Showing nodes accounting for 448.41MB, 98.03% of 457.44MB total
Dropped 60 nodes (cum <= 2.29MB)
Showing top 20 nodes out of 44
      flat  flat%   sum%        cum   cum%
     221MB 48.31% 48.31%   222.50MB 48.64%  github.com/dgraph-io/dgraph/xidmap.(*XidMap).AssignUid
   83.20MB 18.19% 66.50%    83.20MB 18.19%  github.com/dgraph-io/badger/v2/skl.newArena
   65.50MB 14.32% 80.82%    65.50MB 14.32%  strings.(*Builder).WriteString
   38.16MB  8.34% 89.16%    38.16MB  8.34%  bytes.makeSlice
    8.67MB  1.90% 91.06%     8.67MB  1.90%  github.com/dgraph-io/badger/v2/table.(*Builder).addHelper
    7.42MB  1.62% 92.68%    10.42MB  2.28%  github.com/dgraph-io/badger/v2/pb.(*TableIndex).Unmarshal
    6.59MB  1.44% 94.12%     6.59MB  1.44%  github.com/dgraph-io/dgraph/protos/pb.(*PostingList).Marshal
    6.04MB  1.32% 95.44%     6.04MB  1.32%  github.com/dgraph-io/ristretto/z.(*Bloom).Size
       5MB  1.09% 96.53%        5MB  1.09%  github.com/dgraph-io/dgraph/dgraph/cmd/bulk.(*mapIterator).Next
       3MB  0.66% 97.19%        3MB  0.66%  github.com/dgraph-io/badger/v2/pb.(*BlockOffset).Unmarshal
    1.22MB  0.27% 97.46%     6.22MB  1.36%  github.com/dgraph-io/dgraph/dgraph/cmd/bulk.(*reducer).reduce
    1.11MB  0.24% 97.70%     8.20MB  1.79%  github.com/dgraph-io/dgraph/dgraph/cmd/bulk.(*reducer).toList
       1MB  0.22% 97.92%    33.51MB  7.33%  github.com/dgraph-io/badger/v2/table.(*Builder).finishBlock
    0.50MB  0.11% 98.03%     6.65MB  1.45%  github.com/dgraph-io/badger/v2.(*StreamWriter).Write
         0     0% 98.03%    38.16MB  8.34%  bytes.(*Buffer).Write
         0     0% 98.03%    38.16MB  8.34%  bytes.(*Buffer).grow
         0     0% 98.03%    83.70MB 18.30%  github.com/dgraph-io/badger/v2.(*DB).dropAll
         0     0% 98.03%    83.70MB 18.30%  github.com/dgraph-io/badger/v2.(*StreamWriter).Prepare
         0     0% 98.03%     6.15MB  1.35%  github.com/dgraph-io/badger/v2.(*logFile).encodeEntry
         0     0% 98.03%    42.18MB  9.22%  github.com/dgraph-io/badger/v2.(*sortedWriter).Add

This PR:

(pprof) top20
Showing nodes accounting for 154.94MB, 98.96% of 156.57MB total
Dropped 18 nodes (cum <= 0.78MB)
Showing top 20 nodes out of 46
      flat  flat%   sum%        cum   cum%
   83.20MB 53.14% 53.14%    83.20MB 53.14%  github.com/dgraph-io/badger/v2/skl.newArena
   36.53MB 23.33% 76.47%    36.53MB 23.33%  bytes.makeSlice
    8.67MB  5.54% 82.01%     8.67MB  5.54%  github.com/dgraph-io/badger/v2/table.(*Builder).addHelper
    6.04MB  3.86% 85.87%     6.04MB  3.86%  github.com/dgraph-io/ristretto/z.(*Bloom).Size
    5.71MB  3.65% 89.52%     8.71MB  5.56%  github.com/dgraph-io/badger/v2/pb.(*TableIndex).Unmarshal
    5.60MB  3.57% 93.09%     5.60MB  3.57%  github.com/dgraph-io/dgraph/protos/pb.(*PostingList).Marshal
       3MB  1.92% 95.01%        3MB  1.92%  github.com/dgraph-io/badger/v2/pb.(*BlockOffset).Unmarshal
    2.03MB  1.30% 96.31%     2.03MB  1.30%  bufio.NewReaderSize
    1.50MB  0.96% 97.26%        2MB  1.28%  github.com/dgraph-io/dgraph/dgraph/cmd/bulk.(*mapIterator).Next
    1.07MB  0.68% 97.95%     1.07MB  0.68%  github.com/blevesearch/bleve/geo.init.0
    1.03MB  0.66% 98.61%     1.03MB  0.66%  compress/flate.(*dictDecoder).init
    0.54MB  0.35% 98.96%     2.54MB  1.63%  github.com/dgraph-io/dgraph/dgraph/cmd/bulk.(*reducer).reduce
         0     0% 98.96%    36.53MB 23.33%  bytes.(*Buffer).Write
         0     0% 98.96%    36.53MB 23.33%  bytes.(*Buffer).grow
         0     0% 98.96%     1.03MB  0.66%  compress/flate.NewReader
         0     0% 98.96%     1.03MB  0.66%  compress/gzip.(*Reader).Reset
         0     0% 98.96%     1.03MB  0.66%  compress/gzip.(*Reader).readHeader
         0     0% 98.96%     1.03MB  0.66%  compress/gzip.NewReader
         0     0% 98.96%    83.20MB 53.14%  github.com/dgraph-io/badger/v2.(*DB).dropAll
         0     0% 98.96%    83.20MB 53.14%  github.com/dgraph-io/badger/v2.(*StreamWriter).Prepare

I also ran bulk loader on freebase dataset.
System Conf: ubuntu18, 16 core CPU, 64 GB RAM
Datasize: compressed 31GB, ~3.13B RDF

Master:
Time for completion: could not complete, crashed in reduce phase after running for ~2min.
Map phase time: ~1h53m
Reduce phase time:
Peak Memory usage(RES):

This PR:
Time for completion: ~03h40m (successful completion)
Map phase time: ~1h55m
Reduce phase time: ~1h45m
Peak Memory usage(RES): ~56 GB


This change is Reviewable

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @animesh2049 and @manishrjain)

Copy link
Contributor

@animesh2049 animesh2049 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm_strong:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @animesh2049 and @manishrjain)

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@ashish-goswami ashish-goswami merged commit 68ab4c4 into master Feb 6, 2020
@ashish-goswami ashish-goswami deleted the ashish/xidmap-gc branch February 6, 2020 16:19
ahsanbarkati added a commit that referenced this pull request Dec 17, 2020
Fix memory held by b+ tree even in the reduce phase of bulk loader by bringing 
back the change #4738.
danielmai pushed a commit that referenced this pull request Dec 21, 2020
Fix memory held by b+ tree even in the reduce phase of bulk loader by bringing 
back the change #4738.
ahsanbarkati added a commit that referenced this pull request Jan 19, 2021
Fix memory held by b+ tree even in the reduce phase of bulk loader by bringing
back the change #4738.

(cherry picked from commit cc3dbfa)
NamanJain8 pushed a commit that referenced this pull request Jan 19, 2021
Fix memory held by b+ tree even in the reduce phase of bulk loader by bringing
back the change #4738.

(cherry picked from commit cc3dbfa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants