1149272 - Deallocating lots of pipes is slow

Status: NEW

(Core :: Networking, defect, P5)

Description

[Boris Zbarsky [:bzbarsky]]

See the testcase in bug 1149235.

On that testcase, we're spending over 1s under ~nsPipe for all those data: channels (triggered via nsInputStreamReadyEvent::Release releasing the pipe input stream, which release the pipe.

All of this time is under arena_dalloc.  A bunch of it is under _platform_bzero and the rest under arena_run_dalloc/arena_purge/etc.

At a guess, this is the memory being freed by nsSegmentedBuffer, but I'm not sure...

In any case, we're talking about 100000 pipes here, so 1s means 10us per destructor.  That's a fair amount of time...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Er, I meant bug 1148897.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Isn't this a direct trade off of doing a single large allocation and many small segmented allocations?

In theory the segments should get free'd as the data is read from the pipe, though.  Do you expect pipes with a lot of unread data left in them in that test?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Nope.  That test should have nothing of the sort.  The pipes involved should be tiny. They all come from this bit:

    a.src = "data:text/javascript,++j";

so they contain 3 bytes each.  And they get read in their entirety, of course.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I guess we trigger a minimum of two deallocations for every ~nsSegmentBuffer:

  if (mSegmentArray) {
    for (uint32_t i = 0; i < mSegmentArrayCount; i++) {
      if (mSegmentArray[i]) {
        moz_free(mSegmentArray[i]);
      }
    }
    nsMemory::Free(mSegmentArray);
    mSegmentArray = nullptr;
  }

Also, it looks like nsMemory::Free() goes through a function pointer vs xpcomFunctions.freeFunc().

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Do you have a Cleopatra link?

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Seems like ~nsSegmentBuffer could just iterate from mFirstIndexBuffer to mLastIndexBuffer instead of the entire mSegmentArray.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Do you have a Cleopatra link?

No, this was an Instruments profile.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Add a test to benchmark pipe destruction.

Here is the benchmark I wrote to try to figure out what works or not.

Boris, for reference, what does your machine get on this test?

  ./mach gtest Pipes.DestroyPerf

Thanks.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8586183 [details] [diff] [review]
P1 Add a test to benchmark pipe destruction.

Pipes.DestroyPerf took total:172786.342000us mean:1.727863us
Pipes.DestroyPerf took total:185193.900000us mean:1.851939us
Pipes.DestroyPerf took total:172961.337000us mean:1.729613us
Pipes.DestroyPerf took total:167264.762000us mean:1.672648us
Pipes.DestroyPerf took total:171641.188000us mean:1.716412us

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

These results are pretty different from 10us referenced in comment 0.  This seems to suggest there is something else slowing down the deallocations.  Perhaps the other test fragments the heap quite a bit.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

I'm not actively working this.  I'm going to upload the patches wip patch I had to get it out of my local queue.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Add a test to benchmark pipe destruction.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Attachment: bug1149272_p2_reduce_work.patch

Comment 14

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 15

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 16

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=3 years untouched DOM/Storage bug's priority.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.