C5 icon indicating copy to clipboard operation
C5 copied to clipboard

Unecessary allocations in HashSet<T>

Open jnyrup opened this issue 2 years ago • 7 comments

Description

HashSet<T> contains three calls to string.Format in non-invariant checking methods. https://github.com/sestoft/C5/blob/21e90a22a4e75e6fe2d70d2c195b79830d5694ca/C5/Hashing/HashSet.cs#L127 https://github.com/sestoft/C5/blob/21e90a22a4e75e6fe2d70d2c195b79830d5694ca/C5/Hashing/HashSet.cs#L144 https://github.com/sestoft/C5/blob/21e90a22a4e75e6fe2d70d2c195b79830d5694ca/C5/Hashing/HashSet.cs#L167

Even though Logger.Log per default sends any string into void, this still results in some unnecessary string allocations.

Solution

I would suggest deleting those lines or hide them behind #if DEBUG

Describe alternatives you've considered

Additional context

jnyrup avatar Jul 19 '23 13:07 jnyrup

Hej Jonas

Er du interesseret i at bidrage direkte til vedligehold af C5?

De seneste år er det helt overvejende Rasmus Lystrøm (som formentlig auto-cc’s på denne mail) som har passet C5, da jeg er blevet institutleder og ikke kan holde trit med udviklingen i de 117.000 redskaber og frameworks der omgiver et stykke software i dag.

Peter

On 19 Jul 2023, at 15.52, Jonas Nyrup @.***> wrote:

Description

HashSet<T> contains three calls to string.Format in non-invariant checking methods. https://github.com/sestoft/C5/blob/21e90a22a4e75e6fe2d70d2c195b79830d5694ca/C5/Hashing/HashSet.cs#L127 https://github.com/sestoft/C5/blob/21e90a22a4e75e6fe2d70d2c195b79830d5694ca/C5/Hashing/HashSet.cs#L144 https://github.com/sestoft/C5/blob/21e90a22a4e75e6fe2d70d2c195b79830d5694ca/C5/Hashing/HashSet.cs#L167

Even though Logger.Log per default sends any string into void, this still results in some unnecessary string allocations.

Solution

I would suggest deleting those lines or hide them behind #if DEBUG

Describe alternatives you've considered Additional context

— Reply to this email directly, view it on GitHubhttps://github.com/sestoft/C5/issues/138, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAFSQEEKDOGBPIV5MYJW7WLXQ7RDHANCNFSM6AAAAAA2P64DFM. You are receiving this because you are subscribed to this thread.Message ID: @.***>

sestoft avatar Jul 20 '23 09:07 sestoft

Hej Peter

Det kunne jeg godt være interesseret i. For en god ordens skyld skal jeg lige have clearet det med min arbejdsgiver. De rette skal lige have afsluttet deres sommerferier, men jeg vender tilbage, når jeg har et svar.

jnyrup avatar Jul 22 '23 18:07 jnyrup

Hej Peter og Rasmus

Så har jeg fået afklaring og jeg vil gerne hjælpe til med vedligeholdelsen af C5.

jnyrup avatar Aug 25 '23 10:08 jnyrup

Det ser ut som att dokumentationen har genererats felaktigt. Kanske du kan ta en titt på det också?

dlidstrom avatar Aug 28 '23 11:08 dlidstrom

@dlidstrom opret venligst et separat GH issue til dette, det gør det meget lettere at holde styr på.

jnyrup avatar Aug 28 '23 13:08 jnyrup

Hej Jonas,

On 25 Aug 2023, at 12.03, Jonas Nyrup @.***> wrote:

Så har jeg fået afklaring og jeg vil gerne hjælpe til med vedligeholdelsen af C5.

Super.

Nu har Rasmus Lystrøm og jeg også langt om længe mødtes om det, og er enige om at det er meget fint hvis du bliver maintainer på C5.

Der er forskellige mere eller mindre praktiske forhold som skal ordnes (det har Rasmus mere forstand på).

Vi foreslår at mødes alle tre efter 23/10 - online (Teams eller Zoom) med mindre du tilfældigvis er i København alligevel - det står os ikke så klart om du til daglig er i Odense, Danmark, Seattle eller et helt andet sted. Vi er på Amager…

Peter

sestoft avatar Oct 12 '23 12:10 sestoft