Using #function causes big memory leak

Edward_Connell · August 27, 2017, 5:56pm

I reported this about a year ago, but it has never been fixed and it seems
like it should be fixed for the Swift 4.0 release.

Here is a simple repro case. If you watch the memory monitor as it runs,
you see memory consumption climb to 2.7GB when using #function, and no
memory increase when using a static string.

import Foundation

class A {
var counter = 0 {
// didSet { onSet("counter") } // no leak
didSet { onSet() } // huge leak
}

var properties = ["counter" : 0]
func onSet(_ name: String = #function) {
properties[name]! += 1
}
}

var myclass = A()

for i in 0..<10000000 {
myclass.counter = i
}

print(myclass.properties["counter"]!)

Edward_Connell · August 28, 2017, 7:27am

Hi Peter, thanks for taking a look.

I would expect the compiler should generate a read only static string with
the name of the function whenever the function directive is used. I would
expect that should not cause a leak.

If I modify the onSet function to replace a static string as the default
value instead of using function, there is no leak, so this seems like a
bug to me.

Anyone have other ideas or opinions on this?

Thanks, Ed

···

--------------------------------------------------------
import Foundation

class A {
var counter = 0 {
// didSet { onSet("counter") } // no leak
didSet { onSet() } // huge leak
}

var properties = ["counter" : 0]
// func onSet(_ name: String = function) { // leaks
// properties[name]! += 1
// }
func onSet(_ name: String = "counter") { // no leak
properties[name]! += 1
}
}

var myclass = A()

for i in 0..<10000000 {
myclass.counter = i
}

print(myclass.properties["counter"]!)

On Sun, Aug 27, 2017 at 6:59 PM, Peter Nicholls < swiftuser@peternicholls.co.uk> wrote:

I looked in to the memory alloc and every time the “leak” iterates it’s
creating an string item on the auto release stack with “counter” - a few
million of them, compared to just 1 string on the non- leak. It is also
quite a lot slower than the non leak version.

I looked at the compiled assembly intermediate and at allocation the only
difference is that the non-leak allocates via a register and is just a
byte, but the leak uses a memory pointer is a word and is _unnammed_.

My thought is that this isn’t a bug at all, it’s how the compiler deals
with an unnamed function and thus infers _name.

As i trawled through the assembly seemed to me the non leak is making
efficient use of the registers, through the named function / _name, where
as the leak version is using actual memory (hence why it is so much slower)
with a view to destroying the autocomplete stack (of 9million odd auto
release objects at 8bytes at a time) when completed. You can see how the
memory stacks!

Now. As to why and IF Swift is INTENDED to do this, or if indeed you’re
doing something unintended too.... I cannot say.

If any of the above is incorrect, please chime in.

Peter Nicholls
First time post.

> On 27 Aug 2017, at 18:56, Edward Connell via swift-users < > swift-users@swift.org> wrote:
>
> I reported this about a year ago, but it has never been fixed and it
seems like it should be fixed for the Swift 4.0 release.
>
> Here is a simple repro case. If you watch the memory monitor as it runs,
you see memory consumption climb to 2.7GB when using function, and no
memory increase when using a static string.
>
> import Foundation
>
> class A {
> var counter = 0 {
> // didSet { onSet("counter") } // no leak
> didSet { onSet() } // huge leak
> }
>
> var properties = ["counter" : 0]
> func onSet(_ name: String = function) {
> properties[name]! += 1
> }
> }
>
> var myclass = A()
>
> for i in 0..<10000000 {
> myclass.counter = i
> }
>
> print(myclass.properties["counter"]!)
>
> _______________________________________________
> swift-users mailing list
> swift-users@swift.org
> https://lists.swift.org/mailman/listinfo/swift-users

Joe_Groff · August 28, 2017, 3:36pm

I reported this about a year ago, but it has never been fixed and it seems like it should be fixed for the Swift 4.0 release.

What was the SR or radar number?

-Joe

···

On Aug 27, 2017, at 10:57 AM, Edward Connell via swift-users <swift-users@swift.org> wrote:

Here is a simple repro case. If you watch the memory monitor as it runs, you see memory consumption climb to 2.7GB when using function, and no memory increase when using a static string.

import Foundation

class A {
  var counter = 0 {
// didSet { onSet("counter") } // no leak
    didSet { onSet() } // huge leak
  }

  var properties = ["counter" : 0]
  func onSet(_ name: String = function) {
    properties[name]! += 1
  }
}

var myclass = A()

for i in 0..<10000000 {
  myclass.counter = i
}

print(myclass.properties["counter"]!)

_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users

Joe_Groff · August 28, 2017, 3:45pm

The only code generation difference I see, if I modify your didSet to make both calls is that the compiler treats function as a UTF-16 literal, whereas "counter" is recognized as an ASCII literal:

// A.counter.didset
sil hidden @_T03foo1AC7counterSifW : $@convention(method) (Int, @guaranteed A) -> () {
// %0 // user: %2
// %1 // users: %19, %12, %11, %4, %3
bb0(%0 : $Int, %1 : $A):
  debug_value %0 : $Int, let, name "oldValue", argno 1 // id: %2
  debug_value %1 : $A, let, name "self", argno 2 // id: %3
  %4 = class_method %1 : $A, #A.onSet!1 : (A) -> (String) -> (), $@convention(method) (@owned String, @guaranteed A) -> () // user: %11
  %5 = string_literal utf8 "counter" // user: %10
  %6 = integer_literal $Builtin.Word, 7 // user: %10
  %7 = integer_literal $Builtin.Int1, -1 // user: %10
  %8 = metatype $@thin String.Type // user: %10
  // function_ref String.init(_builtinStringLiteral:utf8CodeUnitCount:isASCII:)
  %9 = function_ref @_T0S2SBp21_builtinStringLiteral_Bw17utf8CodeUnitCountBi1_7isASCIItcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String // user: %10
  %10 = apply %9(%5, %6, %7, %8) : $@convention(method) (Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) -> @owned String // user: %11
  %11 = apply %4(%10, %1) : $@convention(method) (@owned String, @guaranteed A) -> ()
  %12 = class_method %1 : $A, #A.onSet!1 : (A) -> (String) -> (), $@convention(method) (@owned String, @guaranteed A) -> () // user: %19
  %13 = string_literal utf16 "counter" // user: %18
  %14 = integer_literal $Builtin.Word, 7 // user: %18
  %15 = integer_literal $Builtin.Int1, -1
  %16 = metatype $@thin String.Type // user: %18
  // function_ref String.init(_builtinUTF16StringLiteral:utf16CodeUnitCount:)
  %17 = function_ref @_T0S2SBp26_builtinUTF16StringLiteral_Bw18utf16CodeUnitCounttcfC : $@convention(method) (Builtin.RawPointer, Builtin.Word, @thin String.Type) -> @owned String // user: %18
  %18 = apply %17(%13, %14, %16) : $@convention(method) (Builtin.RawPointer, Builtin.Word, @thin String.Type) -> @owned String // user: %19
  %19 = apply %12(%18, %1) : $@convention(method) (@owned String, @guaranteed A) -> ()
  %20 = tuple () // user: %21
  return %20 : $() // id: %21
} // end sil function '_T03foo1AC7counterSifW'

Michael, could there be a leak in the implementation of String(_builtinUTF16StringLiteral:utf16CodeUnitCount:)? The SIL at first glance looks balanced here.

-Joe

···

On Aug 27, 2017, at 10:57 AM, Edward Connell via swift-users <swift-users@swift.org> wrote:

import Foundation

class A {
  var counter = 0 {
// didSet { onSet("counter") } // no leak
    didSet { onSet() } // huge leak
  }

  var properties = ["counter" : 0]
  func onSet(_ name: String = function) {
    properties[name]! += 1
  }
}

var myclass = A()

for i in 0..<10000000 {
  myclass.counter = i
}

print(myclass.properties["counter"]!)

Edward_Connell · August 28, 2017, 6:07pm

I reported it 5/16 in bug reporter. Possibly that was the wrong DB?

https://bugreport.apple.com/web/?problemID=26535526

···

On Mon, Aug 28, 2017 at 8:45 AM, Joe Groff <jgroff@apple.com> wrote:

> On Aug 27, 2017, at 10:57 AM, Edward Connell via swift-users < > swift-users@swift.org> wrote:
>
> import Foundation
>
> class A {
> var counter = 0 {
> // didSet { onSet("counter") } // no leak
> didSet { onSet() } // huge leak
> }
>
> var properties = ["counter" : 0]
> func onSet(_ name: String = function) {
> properties[name]! += 1
> }
> }
>
> var myclass = A()
>
> for i in 0..<10000000 {
> myclass.counter = i
> }
>
> print(myclass.properties["counter"]!)

The only code generation difference I see, if I modify your didSet to make
both calls is that the compiler treats function as a UTF-16 literal,
whereas "counter" is recognized as an ASCII literal:

// A.counter.didset
sil hidden @_T03foo1AC7counterSifW : $@convention(method) (Int,
@guaranteed A) -> () {
// %0 // user: %2
// %1 // users: %19, %12, %11,
%4, %3
bb0(%0 : $Int, %1 : $A):
  debug_value %0 : $Int, let, name "oldValue", argno 1 // id: %2
  debug_value %1 : $A, let, name "self", argno 2 // id: %3
  %4 = class_method %1 : $A, #A.onSet!1 : (A) -> (String) -> (),
$@convention(method) (@owned String, @guaranteed A) -> () // user: %11
  %5 = string_literal utf8 "counter" // user: %10
  %6 = integer_literal $Builtin.Word, 7 // user: %10
  %7 = integer_literal $Builtin.Int1, -1 // user: %10
  %8 = metatype $@thin String.Type // user: %10
  // function_ref String.init(_builtinStringLiteral:
utf8CodeUnitCount:isASCII:)
  %9 = function_ref @_T0S2SBp21_builtinStringLiteral_
Bw17utf8CodeUnitCountBi1_7isASCIItcfC : $@convention(method)
(Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) ->
@owned String // user: %10
  %10 = apply %9(%5, %6, %7, %8) : $@convention(method)
(Builtin.RawPointer, Builtin.Word, Builtin.Int1, @thin String.Type) ->
@owned String // user: %11
  %11 = apply %4(%10, %1) : $@convention(method) (@owned String,
@guaranteed A) -> ()
  %12 = class_method %1 : $A, #A.onSet!1 : (A) -> (String) -> (),
$@convention(method) (@owned String, @guaranteed A) -> () // user: %19
  %13 = string_literal utf16 "counter" // user: %18
  %14 = integer_literal $Builtin.Word, 7 // user: %18
  %15 = integer_literal $Builtin.Int1, -1
  %16 = metatype $@thin String.Type // user: %18
  // function_ref String.init(_builtinUTF16StringLiteral:
utf16CodeUnitCount:)
  %17 = function_ref @_T0S2SBp26_builtinUTF16StringLiteral_Bw18utf16CodeUnitCounttcfC
: $@convention(method) (Builtin.RawPointer, Builtin.Word, @thin
String.Type) -> @owned String // user: %18
  %18 = apply %17(%13, %14, %16) : $@convention(method)
(Builtin.RawPointer, Builtin.Word, @thin String.Type) -> @owned String //
user: %19
  %19 = apply %12(%18, %1) : $@convention(method) (@owned String,
@guaranteed A) -> ()
  %20 = tuple () // user: %21
  return %20 : $() // id: %21
} // end sil function '_T03foo1AC7counterSifW'

Michael, could there be a leak in the implementation of String(_
builtinUTF16StringLiteral:utf16CodeUnitCount:)? The SIL at first glance
looks balanced here.

-Joe

Joe_Groff · August 28, 2017, 6:09pm

No, that's correct. Thanks!

-Joe

···

On Aug 28, 2017, at 11:07 AM, Edward Connell <ewconnell@gmail.com> wrote:

I reported it 5/16 in bug reporter. Possibly that was the wrong DB?

Feedback Assistant <Feedback Assistant;