Skip to content
This repository was archived by the owner on Dec 20, 2024. It is now read-only.

Conversation

@lowzj
Copy link
Member

@lowzj lowzj commented Jun 2, 2020

Signed-off-by: lowzj [email protected]

Ⅰ. Describe what this PR did

This pull request uses sync.Pool to manager the bytes.Buffer and bufio.Writer to minimizing the memory allocation and usage.

Here is a comparison when downloading a 10GB file.

before:

Type: alloc_space
Time: Jul 1, 2020 at 12:53am (CST)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 14.44GB, 99.94% of 14.45GB total
Dropped 74 nodes (cum <= 0.07GB)
Showing top 10 nodes out of 11
      flat  flat%   sum%        cum   cum%
   12.85GB 88.94% 88.94%    12.85GB 88.94%  bytes.makeSlice
    1.59GB 11.00% 99.94%     1.59GB 11.00%  bufio.NewWriterSize (inline)
         0     0% 99.94%    12.85GB 88.94%  bytes.(*Buffer).ReadFrom
         0     0% 99.94%    12.85GB 88.94%  bytes.(*Buffer).grow
         0     0% 99.94%     1.59GB 11.00%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*ClientWriter).Run
         0     0% 99.94%     1.59GB 11.00%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*ClientWriter).write
         0     0% 99.94%     1.59GB 11.00%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*P2PDownloader).run.func1
         0     0% 99.94%    12.86GB 88.95%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*P2PDownloader).startTask
         0     0% 99.94%    12.86GB 88.95%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*PowerClient).Run
         0     0% 99.94%    12.86GB 88.95%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*PowerClient).downloadPiece
Type: inuse_space
Time: Jul 1, 2020 at 12:52am (CST)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 6.34GB, 100% of 6.34GB total
Dropped 10 nodes (cum <= 0.03GB)
      flat  flat%   sum%        cum   cum%
    6.34GB   100%   100%     6.34GB   100%  bytes.makeSlice
         0     0%   100%     6.34GB   100%  bytes.(*Buffer).ReadFrom
         0     0%   100%     6.34GB   100%  bytes.(*Buffer).grow
         0     0%   100%     6.34GB   100%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*P2PDownloader).startTask
         0     0%   100%     6.34GB   100%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*PowerClient).Run
         0     0%   100%     6.34GB   100%  github.com/dragonflyoss/Dragonfly/dfget/core/downloader/p2p_downloader.(*PowerClient).downloadPiece

after:

Type: alloc_space
Time: Jul 1, 2020 at 12:44am (CST)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 129.47MB, 97.72% of 132.50MB total
Dropped 31 nodes (cum <= 0.66MB)
Showing top 10 nodes out of 33
      flat  flat%   sum%        cum   cum%
     120MB 90.57% 90.57%      120MB 90.57%  github.com/dragonflyoss/Dragonfly/pkg/pool.NewBuffer
    3.97MB  3.00% 93.56%     3.97MB  3.00%  bufio.NewWriterSize
       2MB  1.51% 95.07%        2MB  1.51%  github.com/go-openapi/swag.(*NameProvider).GetJSONNames
       2MB  1.51% 96.58%        2MB  1.51%  reflect.mapassign
       1MB  0.75% 97.34%     1.50MB  1.13%  encoding/json.(*decodeState).objectInterface
    0.50MB  0.38% 97.72%     6.50MB  4.91%  github.com/go-openapi/spec.(*Schema).UnmarshalJSON
         0     0% 97.72%     6.50MB  4.91%  encoding/json.(*decodeState).object
         0     0% 97.72%     6.50MB  4.91%  encoding/json.(*decodeState).unmarshal
         0     0% 97.72%     6.50MB  4.91%  encoding/json.(*decodeState).value
         0     0% 97.72%        1MB  0.75%  encoding/json.(*decodeState).valueInterface
Type: inuse_space
Time: Jul 1, 2020 at 12:43am (CST)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 94.84MB, 100% of 94.84MB total
Showing top 10 nodes out of 48
      flat  flat%   sum%        cum   cum%
      90MB 94.90% 94.90%       90MB 94.90%  github.com/dragonflyoss/Dragonfly/pkg/pool.NewBuffer
    2.81MB  2.97% 97.87%     2.81MB  2.97%  bufio.NewWriterSize
    0.52MB  0.55% 98.42%     0.52MB  0.55%  regexp/syntax.(*compiler).inst
    0.50MB  0.53% 98.95%     0.50MB  0.53%  regexp.onePassCopy
    0.50MB  0.53% 99.47%     0.50MB  0.53%  reflect.mapassign
    0.50MB  0.53%   100%     0.50MB  0.53%  github.com/spf13/pflag.(*FlagSet).VarPF
         0     0%   100%     0.50MB  0.53%  encoding/json.(*decodeState).object
         0     0%   100%     0.50MB  0.53%  encoding/json.(*decodeState).unmarshal
         0     0%   100%     0.50MB  0.53%  encoding/json.(*decodeState).value
         0     0%   100%     0.50MB  0.53%  encoding/json.Unmarshal

Ⅱ. Does this pull request fix one issue?

fixes: #1206

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added areas/test kind/bug This is bug report for project size/S labels Jun 2, 2020
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch from 5a69490 to f271131 Compare June 2, 2020 11:02
@lowzj lowzj changed the title bugfix: release allocated ByteBuffer explicitly [WIP] bugfix: release allocated ByteBuffer explicitly Jun 2, 2020
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch from f271131 to d6a55e7 Compare June 2, 2020 11:12
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch from d6a55e7 to 6148c44 Compare June 30, 2020 08:56
@pouchrobot pouchrobot added size/L and removed size/S labels Jun 30, 2020
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch 2 times, most recently from 45e1063 to c37d86b Compare June 30, 2020 09:15
@pouchrobot pouchrobot added size/XL and removed size/L labels Jun 30, 2020
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch from c37d86b to b8f203e Compare June 30, 2020 12:34
@lowzj lowzj changed the title [WIP] bugfix: release allocated ByteBuffer explicitly bugfix: release allocated ByteBuffer explicitly Jun 30, 2020
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch 3 times, most recently from daf9f17 to 77b1d60 Compare June 30, 2020 17:25
@lowzj lowzj requested a review from starnop June 30, 2020 17:31
@lowzj
Copy link
Member Author

lowzj commented Jun 30, 2020

@starnop PTAL

@lowzj lowzj changed the title bugfix: release allocated ByteBuffer explicitly optimize: release allocated ByteBuffer explicitly Jul 1, 2020
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch from 77b1d60 to cf69efd Compare July 1, 2020 03:21
@codecov-commenter
Copy link

Codecov Report

Merging #1375 into master will increase coverage by 0.20%.
The diff coverage is 73.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1375      +/-   ##
==========================================
+ Coverage   52.02%   52.22%   +0.20%     
==========================================
  Files         137      139       +2     
  Lines        9108     9181      +73     
==========================================
+ Hits         4738     4795      +57     
- Misses       3987     4001      +14     
- Partials      383      385       +2     
Impacted Files Coverage Δ
...t/core/downloader/p2p_downloader/p2p_downloader.go 2.11% <0.00%> (ø)
dfget/core/downloader/p2p_downloader/piece.go 21.81% <4.76%> (-6.39%) ⬇️
...get/core/downloader/p2p_downloader/power_client.go 50.98% <66.66%> (ø)
...et/core/downloader/p2p_downloader/client_writer.go 8.66% <100.00%> (+0.72%) ⬆️
pkg/pool/buffer_pool.go 100.00% <100.00%> (ø)
pkg/pool/writer_pool.go 100.00% <100.00%> (ø)
supernode/daemon/mgr/scheduler/manager.go 21.91% <0.00%> (-0.69%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5cc28f...cf69efd. Read the comment docs.

@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch 3 times, most recently from 44a5afc to 7b3d5f8 Compare July 1, 2020 06:31
use sync.Pool to manager the bytes.Buffer and bufio.Writer
to minimizing the memory allocation and usage

Signed-off-by: lowzj <[email protected]>
@lowzj lowzj force-pushed the fix-ByteBuffer-gc branch from 7b3d5f8 to 764b4ed Compare July 1, 2020 07:15
@starnop
Copy link
Contributor

starnop commented Jul 2, 2020

LGTM.

@starnop starnop merged commit 88cb000 into dragonflyoss:master Jul 2, 2020
lowzj added a commit that referenced this pull request Jul 2, 2020
optimize: release allocated ByteBuffer explicitly
@lowzj lowzj deleted the fix-ByteBuffer-gc branch July 16, 2020 07:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

areas/test kind/bug This is bug report for project size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dfget is downloading large file (5GB), and out of memory or killed (trigger system OOM) appears

4 participants