Performance regression in NSMutableData[SR-3966] #890
Conversation
|
|
||
| __CFDataSetNumBytesUsed(memory, 0); | ||
| __CFDataSetLength(memory, 0); | ||
| __CFDataSetInfoBits(memory, __kCFDontDeallocate); | ||
| if(isDontDeallocate) { |
There was a problem hiding this comment.
Nit: space after if, like line 323.
|
|
||
| __CFDataSetNumBytesUsed(memory, 0); | ||
| __CFDataSetLength(memory, 0); | ||
| __CFDataSetInfoBits(memory, __kCFDontDeallocate); | ||
| if(isDontDeallocate) { | ||
| __CFDataSetInfoBits(memory, __kCFDontDeallocate); |
There was a problem hiding this comment.
Nit: Should be 4 spaces indentation.
|
@phausler please review |
saiHemak
force-pushed
the
nsmutabledata-branch
branch
from
6b8f165 to
a39d6c3
Compare
|
@ianpartridge Thanks for your comments .I have addressed it |
|
To be clear, this isn't a performance regression. It's a memory leak. It was fixed in #380 and overwritten in the Sierra merge #709 @parkera There seem to have been a few issues like this. The libdispatch folks tag PRs as |
|
@swift-ci test and merge |
|
@saiHemak can you put up a PR for the swift-3.1 branch as well? |
|
@ianpartridge I'll look into why this one was overwritten. The process we are following is to merge all open source changes back, then merge from the internal branch back to open source. Clearly something got missed here. |
|
@swift-ci test and merge |
|
I think there was merge damage here; iirc we had a fix that was applied in both sides of the open source merge from Sierra that perhaps caused this to get clobbered |
PR which addressed the memory leak in NSMutableData through #380 is missing in the latest code as well as the Swift-3.1 branch.Hence it resulted in the regression with latest code base.Please merge this on priority .