PieterTack / polycap

Polycapillary X-ray raytracing
GNU General Public License v3.0
3 stars 3 forks source link

leak_calc debug: initial commit #46

Closed PieterTack closed 5 years ago

PieterTack commented 5 years ago

issue #44

PieterTack commented 5 years ago

It appears there is no mem leak (any more?) in polycap_photon_launch() or polycap_capil_reflect() for two special cases (still need to make test for third leak option, which happens to be the most difficult and probably error prone) Still running Valgrind to check polycap_photon_get_transmission_efficiencies(), but it's looking hopeful (at least we're narrowing down to the issue :) )

tschoonj commented 5 years ago

Good!

To fix the compilation errors on macOS and Windows use PRId64 in your call to printf, just as in https://github.com/PieterTack/polycap/blob/9608af9c2afb0f3bc4235535b7c0ba23add4cc83/src/polycap-source.c#L880

PieterTack commented 5 years ago

In laatste commit ben ik misschien op het kern-probleem gestuit... Het geval is dus dat in die laatste test die ik aanmaakte (waaronder de printf worden uitgevoerd, waarop ook gecrashed wordt), dat n_recap en n_leaks allebei != 0 zouden moeten zijn. Echter hier zijn ze wel gelijk aan 0, waardoor de photon->leaks en photon->recap structures niet worden allocated, en er dus een fout ontstaat wanneer ik hun interne waarden probeer te lezen.

De core issue zit er hem dus in dat, om een of andere reden de recap en/of leak events niet correct worden geregistreerd in polycap_capil_reflect(), wanneer wall_trace == 1 (lijn 321 e.v. in polycap_capil.c)

De memory leak die ik opspoor lijkt echter niet in polycap_capil_reflect() te zitten, want valgrind op tests/leaks.c geeft geen issues, maar polycap_source_get_transmission_efficiencies() wel...

Heb jij inspiratie wat het probleem precies is?

PieterTack commented 5 years ago

Wat 'progressie'

Heb nu een mogelijke indicatie van bug gevonden in polycap_capil_reflect(). Doe de leaks test en je zal zien wat ik bedoel: polycap_error set over the top of a previous polycap_error or uninitialized memory. ... This overwriting error message was: polycap_capil_trace_wall: photon_pos_check: photon not within polycapillary boundaries

Ik vermoed dus dat waar ik, in de functie polycap_capil_reflect(), de polycap_capil_trace() functie oproep (die op zijn beurt terug polycap_capil_reflect() oproept) met photon_temp, dat er stoute dingen gebeuren. Probleem dat hij nu geeft lijkt te zijn dat hij error overschrijft. Het onderliggende probleem is uiteraard dat ik uberhaupt een error meekrijg, maar kzie niet goed in welke situaties mijn code dergelijke problemen zou kunnen oplopen...

tschoonj commented 5 years ago

Kijk eens naar die compilatie errors op https://travis-ci.org/PieterTack/polycap/jobs/581180534#L2867-L2892

Voor integers gebruik je best abs ipv fabs

PieterTack commented 5 years ago

Tzit nog iets niet juist... Als ik de main code laat draaien, met leak calculation, dan crasht ze nog steeds. Valgrind lijkt geen fouten meer mee te geven in de aparte tests (behalve dan wat memory leak die ik niet goed kan plaatsen, en waarvan ik me meen te herinneren dat ze intrinsiek was aan realloc en/of de multi-core processing. Misschien dat deze nu toch te groot wordt op termijn, met alle leak reallocs?)

PieterTack commented 5 years ago

Nog eens goed naar valgrind gekeken. De leaks die er nog zijn lijken uit polycap-source.c te komen, kijk vooral naar lijnen 765 - 853. Ik vermoed dat het probleem zit in polycap_leak_free() (uit polycap-photon.c). Misschien de frekwentie waarmee ik ze oproep, of dat ik ergens delen van de _polycap_leaks struct vergeet te free'en?

tschoonj commented 5 years ago

Zal er straks eens naar zien

tschoonj commented 5 years ago

Hoe roep je de main op precies?

PieterTack commented 5 years ago

./src/polycap ../example/dub_foc.inp dub_foc.h5 -1 1 In map build (laatste 1 is van leak calc, indien niet wil doen dan gewoon de 1 weglaten)

tschoonj commented 5 years ago

Dat blijft hier maar gaan bij mij... valgrind heb ik ganse nacht laten lopen zonder dat het eindigde.

Nu met gdb al een uur of twee maar nog geen eind in zicht 😄

PieterTack commented 5 years ago

Is op zich al knap dat het blijft gaan bij jou :P Bij mij crasht m'n ganse computer ervan na een dag ofzo :P Het volledige programma draaien met de leak calc aan zal inderdaad zeer lang duren. Kschat dat het zelfs zonder valgrind, met 8 cores, ongeveer 2 uur duurt voor hij klaar is. (khoop dat de linked lists iets sneller zullen gaan enzo op termijn, en anders moet ik eens proberen een schatting te maken van hoeveel memory ik op z'n meest zou nodig hebben, en dan dit in het begin alloc'en en van daar verder...)

Ikzelf doe meestal gewoon de leak tests, maar is uiteraard maar beperkte informatie dan.

tschoonj commented 5 years ago

Ik denk dat jouw programma crasht door een gebrek aan geheugen 😄

Ik veronderstel dat op een bepaald moment malloc of realloc een NULL produceert daardoor waardoor je dan de crash krijgt.

Ik denk dat je best eens kijkt naar de single en double linked lists in GLib en die dan gebruikt.

Momenteel ben ik op reis in India, dus waarschijnlijk niet altijd even bereikbaar...

PieterTack commented 5 years ago

Ik vreesde inderdaad al iets dat ik uiteindelijk zonder geheugen raak :P Khoopte echter dat Valgrind mij dat ook zou zeggen ofzo ;) Kzal inderdaad eens beginnen kijken op de linked lists, want ik krijg het vermoeden dat dat de enige echte werkbare methode zal zijn :)

Er zit echter ook nog een probleem bij de leak tests. Valgrind geeft me daar, onder andere, een deel memory dat lost is (maar geen exacte aanduiding van welke lijn). En is maar met simulatie van 1000 fotonen, dus memory zou geen issue mogen zijn. Misschien dat jij eens gdb kan loslaten op de leaks test?

PieterTack commented 5 years ago

Als ik die linked lists zou beginnen maken, zou ik liefst branchen van deze pull request. Hoe moet ik dat dan doen? Of maak ik beter een nieuwe branch van de origin, en zien we later dan wel hoe we de twee branches/pull requests mergen?

tschoonj commented 5 years ago

Nu ik er aan denk zal linked lists niet het einde van uw problemen betekenen als je elk leak event wilt bijhouden. Performantie zal omhoog gaan omdat realloc nu eenmaal schabouwelijk traag is, maar je zal evenveel geheugen blijven gebruiken.

Samengevat: uw programma zal nog altijd crashen door gebrek aan geheugen, het zal alleen sneller gebeuren 😉

PieterTack commented 5 years ago

Ja, had er ook al aan gedacht, maar dan zal ik sneller kunnen inschatten hoeveel leaks hij genereert enzo. Kan dan mss in een volgende stap kijken om bvb enkel de internal leak/recap events op te slaan e.d. Dat zal niet echt veel sneller processen, maar wel geheugen sparen.

Daarna kan ik eventueel eens kijken om bvb te monitoren hoeveel geheugen beschikbaar is, en als te klein wordt ofzo eerst deel van resultaten wegschrijven (uit geheugen releasen) en dan verder rekenen, maar dat zal wss niet simpel zijn met multicore processing enzo.

Hoe dan ook, denk je dat goed idee is om te branchen van deze pull request? Of beter van origin? Kzou deze PR nog niet mergen, omdat er nog een issue zit bij die leak tests...

Sorry ook dat ik je zoveel stoor nu hoor, zeker nu je op reis ben! Hopelijk weerhou ik je er niet van om van je familie te genieten :)

tschoonj commented 5 years ago

Probeer eens valgrind te laten lopen met opties --leak-check=full --show-leak-kinds=all --track-origins=yes. Volgens mij is er ergens een klein memory leak dat deze opties zouden moeten tonen.

Als je tevreden bent met de commits in deze branch, merge ze dan in, en start te werken met een nieuwe branch voor de linked lists.

PieterTack commented 5 years ago

Ik blijf de memory leak hebben. Valgrind met track-origins helpt niet echt: blijft verwijzen naar de #pragma omp in polycap-source.c Wanneer ik de test single core uitvoer zijn er geen memory leaks, dus kvermoed ergens een memory race ofzo, maar kzie niet in hoe/waar.

In elk geval mss best gewoon mergen met origin, en dan kan ik aan die linked lists werken. Mss lost dat automatisch het probleem op.

tschoonj commented 5 years ago

Kunde eens copy-paste wat uw valgrind output is? Zou kunnen een false positive zijn...

PieterTack commented 5 years ago

Valgrind output: ==9497== Memcheck, a memory error detector ==9497== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==9497== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info ==9497== Command: ./tests/leaks ==9497== 10% Complete 24 reflections Last reflection at z=8.955717, d_travel=8.955802 20% Complete 28 reflections Last reflection at z=8.983694, d_travel=8.984267 31% Complete 21 reflections Last reflection at z=8.993638, d_travel=8.993855 41% Complete 21 reflections Last reflection at z=8.993994, d_travel=8.994245 52% Complete 28 reflections Last reflection at z=8.973594, d_travel=8.973961 62% Complete 29 reflections Last reflection at z=8.992713, d_travel=8.993120 72% Complete 37 reflections Last reflection at z=8.995471, d_travel=8.996153 83% Complete 24 reflections Last reflection at z=8.974279, d_travel=8.974410 93% Complete 32 reflections Last reflection at z=8.912093, d_travel=8.912391 Average number of reflections: 33.273000, Simulated photons: 1856 Open area Calculated: 0.574548, Simulated: 0.538793 ==9497== ==9497== HEAP SUMMARY: ==9497== in use at exit: 7,120 bytes in 11 blocks ==9497== total heap usage: 8,388,617 allocs, 8,388,606 frees, 13,749,927,488 bytes allocated ==9497== ==9497== 8 bytes in 1 blocks are still reachable in loss record 1 of 5 ==9497== at 0x4A0704B: malloc (vg_replace_malloc.c:299) ==9497== by 0x5E85498: gomp_malloc (alloc.c:36) ==9497== by 0x5E8FC47: gomp_init_num_threads (proc.c:90) ==9497== by 0x5E8648C: initialize_env (env.c:1187) ==9497== by 0x5E91AD5: ??? (in /usr/local/gcc49/lib64/libgomp.so.1.0.0) ==9497== by 0x5E85002: ??? (in /usr/local/gcc49/lib64/libgomp.so.1.0.0) ==9497== ==9497== 72 bytes in 1 blocks are still reachable in loss record 2 of 5 ==9497== at 0x4A06F8C: malloc (vg_replace_malloc.c:298) ==9497== by 0x4A0913F: realloc (vg_replace_malloc.c:785) ==9497== by 0x5E854E8: gomp_realloc (alloc.c:54) ==9497== by 0x5E8E73F: gomp_team_start (team.c:445) ==9497== by 0x5E8AA59: GOMP_parallel (parallel.c:166) ==9497== by 0x40AAB7: polycap_source_get_transmission_efficiencies (polycap-source.c:671) ==9497== by 0x405C93: test_polycap_source_leak (leaks.c:519) ==9497== by 0x405D92: main (leaks.c:535) ==9497== ==9497== 192 bytes in 1 blocks are still reachable in loss record 3 of 5 ==9497== at 0x4A0704B: malloc (vg_replace_malloc.c:299) ==9497== by 0x5E85498: gomp_malloc (alloc.c:36) ==9497== by 0x5E8E882: gomp_new_thread_pool (team.c:198) ==9497== by 0x5E8E882: gomp_team_start (team.c:293) ==9497== by 0x5E8AA59: GOMP_parallel (parallel.c:166) ==9497== by 0x40AAB7: polycap_source_get_transmission_efficiencies (polycap-source.c:671) ==9497== by 0x405C93: test_polycap_source_leak (leaks.c:519) ==9497== by 0x405D92: main (leaks.c:535) ==9497== ==9497== 2,816 bytes in 1 blocks are still reachable in loss record 4 of 5 ==9497== at 0x4A0704B: malloc (vg_replace_malloc.c:299) ==9497== by 0x5E85498: gomp_malloc (alloc.c:36) ==9497== by 0x5E8DB59: gomp_new_team (team.c:148) ==9497== by 0x5E8AA45: GOMP_parallel (parallel.c:166) ==9497== by 0x40AAB7: polycap_source_get_transmission_efficiencies (polycap-source.c:671) ==9497== by 0x405C93: test_polycap_source_leak (leaks.c:519) ==9497== by 0x405D92: main (leaks.c:535) ==9497== ==9497== 4,032 bytes in 7 blocks are possibly lost in loss record 5 of 5 ==9497== at 0x4A08EFE: calloc (vg_replace_malloc.c:711) ==9497== by 0x35030119A2: _dl_allocate_tls (in /lib64/ld-2.12.so) ==9497== by 0x35038072CC: pthread_create@@GLIBC_2.2.5 (in /lib64/libpthread-2.12.so) ==9497== by 0x5E8E05C: gomp_team_start (team.c:795) ==9497== by 0x5E8AA59: GOMP_parallel (parallel.c:166) ==9497== by 0x40AAB7: polycap_source_get_transmission_efficiencies (polycap-source.c:671) ==9497== by 0x405C93: test_polycap_source_leak (leaks.c:519) ==9497== by 0x405D92: main (leaks.c:535) ==9497== ==9497== LEAK SUMMARY: ==9497== definitely lost: 0 bytes in 0 blocks ==9497== indirectly lost: 0 bytes in 0 blocks ==9497== possibly lost: 4,032 bytes in 7 blocks ==9497== still reachable: 3,088 bytes in 4 blocks ==9497== suppressed: 0 bytes in 0 blocks ==9497== ==9497== For counts of detected and suppressed errors, rerun with: -v ==9497== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4)

tschoonj commented 5 years ago

Ok, niet aantrekken, die leaks komen vanuit de OpenMP implementatie, en zijn dus niet uw fout. Ze zijn hoe dan ook niet groot, en zullen niet problematisch zijn.

PieterTack commented 5 years ago

Ok :) Geef je dan een positieve review? Dan kan ik mergen en mss morgen al eens kijken voor de linked lists.

PieterTack commented 5 years ago

Hm, net een issue gemerkt: de transmission efficiency simulatie met of zonder leak option geeft verschillende transmissie efficienties voor de niet-leak events.