Performance regression in Buffer.concat #45802

Activity

1. VoltrexKeyva

   added

   bufferIssues and PRs related to the buffer subsystem.Issues and PRs related to the buffer subsystem.

   performanceIssues and PRs related to the performance of Node.js.Issues and PRs related to the performance of Node.js.

   on Dec 10, 2022
2. RafaelGSS commented on Dec 15, 2022

   

   RafaelGSS

   on Dec 15, 2022

   Last edited by RafaelGSS

   Member

   More actions

   HI @wemeetagain. I've run it locally and it seems fixed in v19. See below:

   v14

   concat 10 buffers, 32 each: 0.00033s

   v16

   concat 10 buffers, 32 each: 0.00044s

   v18

   concat 10 buffers, 32 each: 0.00114s

   v19

   concat 10 buffers, 32 each: 0.0004s

   ---

   Note: I haven't investigated the cause, but, I'd wait for the next v18 release and compare.

   👍React with 👍1Reacted by Cayman
3. krk commented on Dec 30, 2022

   

   krk

   on Dec 30, 2022

   Contributor

   More actions

   I bisected with the help of nvm, found that 17.2.0 introduced the regression. Then profiling with perf using this modified reproducer:

   const randomBuffer = (length) => Buffer.from(Array.from({ length }, () => Math.random() * 255))
   
   const ranBuf = randomBuffer(32);
   
   const results = []
   const buffers = Array.from({ length: 10 }, () => ranBuf)
   
   const before = performance.now();
   for (let i = 0; i < 10000000; i++) {
       Buffer.concat(buffers)
   }
   const after = performance.now()
   console.log(after - before);

   17.1.0 (good)

     28,82%  node     node                 [.] Builtins_TypedArrayPrototypeSet                                                                                                                                       
      6,52%  node     node                 [.] Builtins_CreateTypedArray                                                                                                                                             
      4,11%  node     [JIT] tid 6479       [.] 0x00007f307e848d70                                                                                                                                                    
      3,53%  node     [JIT] tid 6479       [.] 0x00007f307e848d2e                                                                                                                                                    
      2,92%  node     libc-2.26.so         [.] __memmove_avx_unaligned_erms                                                                                                                                          
      2,66%  node     node                 [.] Builtins_LoadGlobalIC                                                                                                                                                 
      1,71%  node     libc-2.26.so         [.] _int_malloc                                                                                                                                                           
      1,54%  node     [JIT] tid 6479       [.] 0x00007f307e848d3d                                                                                                                                                    
      1,52%  node     [JIT] tid 6479       [.] 0x00007f307e848a46                                                                                                                                                    
      1,43%  node     libc-2.26.so         [.] malloc
   ...
      0,02%  node  node  [.] Builtins_LoadIC                                                                                                                                                                         
      0,08%  node  node  [.] Builtins_Call_ReceiverIsAny                                                                                                                                                             
      0,00%  node  node  [.] Builtins_TypedArrayPrototypeLength                                                                                                                                                     
      0,81%  node  node  [.] Builtins_CallFunction_ReceiverIsAny                                                                                                                                                     
   
      Builtins_LoadICTrampoline NOT sampled.
   

   17.2.0 (bad)

     32,32%  node     node                 [.] Builtins_LoadIC
     13,60%  node     node                 [.] Builtins_Call_ReceiverIsAny
     12,57%  node     node                 [.] Builtins_TypedArrayPrototypeLength
      9,43%  node     node                 [.] Builtins_CallFunction_ReceiverIsAny
      8,66%  node     node                 [.] Builtins_TypedArrayPrototypeSet
      1,82%  node     node                 [.] Builtins_CreateTypedArray
      1,69%  node     node                 [.] Builtins_LoadICTrampoline
      0,96%  node     [JIT] tid 6491       [.] 0x00007f7ab7048fa0
      0,93%  node     node                 [.] Builtins_LoadGlobalIC
      0,85%  node     libc-2.26.so         [.] __memmove_avx_unaligned_erms
      0,59%  node     libc-2.26.so         [.] _int_malloc
      0,38%  node     libc-2.26.so         [.] malloc
   

   In 17.2.0, Builtins_* dominate the call stacks, pointing to a v8 issue.

   Further bisecting with git identifies #40488, which updates v8 from 9.5.172.25 to 9.6.180.14 as the PR when this regression was introduced.

   👍React with 👍3Reacted by Cayman, Raz Luvaton and Seth Pollack
4. spollack commented on Feb 10, 2023

   

   spollack

   on Feb 10, 2023

   More actions

   Question: is the perf fix likely to be picked up for node 18.x? or will we have to wait for 20.x to have the fix in an LTS? Thank you.
5. wemeetagain commented on Oct 10, 2024

   

   wemeetagain

   on Oct 10, 2024

   Author

   More actions

   appears to be resolved by node 20 (also 22 is good)
6. wemeetagain

   closed this as completedon Oct 10, 2024