[Question] Why isn't `defer` used?

[dharmit]

Hi, great repo. I'm simply reading through the code for fun. :)

I'm curious to understand why there isn't much use of defer to unlock the mutex in threadsafe.go because in most of the thread-safe code I've seen, folks prefer using defer to not mistakenly forget unlocking stuff.

Is it some kind of design pattern/decision that was consciously made while writing the library?

[dharmit]

Ah, I found https://github.com/deckarep/golang-set/pull/14 after opening the issue. I had searched only in the issues, not in PRs. :)

I modified the code to use defer everywhere and found that the performance reason still holds true at few places.

[deckarep]

Hello,

In general I would say to stick to using defer in all typical Go code. But yes, as you have discovered you can still pay a small performance cost for it and especially if the code is in a tight loop.

This library could easily be used in a tight loop so it makes sense to try to make this code as performant as possible (within reason).

Also, since a lot of locking/unlocking is already small in scope (very small functions) it’s easy to see that defer is practically unnecessary anyway.

I hope that helps!

On Sat, Aug 19, 2023 at 9:07 AM Dharmit Shah @.***> wrote:

Closed #125 https://github.com/deckarep/golang-set/issues/125 as completed.

— Reply to this email directly, view it on GitHub https://github.com/deckarep/golang-set/issues/125#event-10136598711, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABQ73QZFP6LQ5V3XNM72XLXWDQFHANCNFSM6AAAAAA3WUXU7A . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[dharmit]

Also, since a lot of locking/unlocking is already small in scope (very small functions) it’s easy to see that defer is practically unnecessary anyway.

Indeed! And this is something I had in the back of my mind while opening the issue, that it's not a big deal to not have defer because of the size of the functions. But I found a few occurrences of defer so asked here nonetheless.

But defer's performance penalty was definitely a TIL for me. :)

[deckarep]

Well said!

On Sat, Aug 19, 2023 at 10:44 PM Dharmit Shah @.***> wrote:

Also, since a lot of locking/unlocking is already small in scope (very small functions) it’s easy to see that defer is practically unnecessary anyway.

Indeed! And this is something I had in the back of my mind while opening the issue, that it's not a big deal to not have defer because of the size of the functions. But I found a few occurrences of defer so asked here nonetheless.

But defer's performance penalty was definitely a TIL for me. :)

— Reply to this email directly, view it on GitHub https://github.com/deckarep/golang-set/issues/125#issuecomment-1685191920, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABQ73VE3RMPJYXTEZDDYALXWGP3RANCNFSM6AAAAAA3WUXU7A . You are receiving this because you commented.Message ID: @.***>

[Darkheir]

I think that since go 1.14 defer doesn't bring any performance penalty:

This release improves the performance of most uses of defer to incur almost zero overhead compared to calling the deferred function directly. As a result, defer can now be used in performance-critical code without overhead concerns.

Anyway this lib was created long before the 1.14 release :)

[deckarep]

I remember this release and the Go team claiming this but later it was actually revealed with more analysis that defer does still have some non-negligible overhead.

At some point I’ll check into this again.