gnudatalanguage / gdl

GDL - GNU Data Language
GNU General Public License v2.0
276 stars 61 forks source link

Shmmap etc #1703

Closed GillesDuvert closed 10 months ago

GillesDuvert commented 10 months ago

WIP but can be tested

brandy125 commented 10 months ago

Very nice!

On 30. Dec 2023, at 16:07, Giloo @.***> wrote:

WIP but can be tested

You can view, comment on, or merge this pull request online at:

https://github.com/gnudatalanguage/gdl/pull/1703

Commit Summary

7620e23 https://github.com/gnudatalanguage/gdl/pull/1703/commits/7620e23be0d911bdd4e057beba3bab3a8f471ee6 install empty shm support 986e9ad https://github.com/gnudatalanguage/gdl/pull/1703/commits/986e9ada272c2e4ac362b2e69d7f84aa1abaa303 e5d4fcd https://github.com/gnudatalanguage/gdl/pull/1703/commits/e5d4fcd0af4eadf726e6cd721d9fe590b0c428bd quite satisfactorily functional 36dae09 https://github.com/gnudatalanguage/gdl/pull/1703/commits/36dae09d2a65166cb335ab6f591d28f1fecd90c8 protect against MSWindows for now. File Changes (4 files https://github.com/gnudatalanguage/gdl/pull/1703/files) M src/CMakeLists.txt https://github.com/gnudatalanguage/gdl/pull/1703/files#diff-148715d6ea0c0ea0a346af3f6bd610d010d490eca35ac6a9b408748f7ca9e3f4 (3) M src/libinit.cpp https://github.com/gnudatalanguage/gdl/pull/1703/files#diff-89715e6964a87b8ff56a15d88ad779434f864f8d8017b8715ed2acf6beeb88e1 (18) A src/shm.cpp https://github.com/gnudatalanguage/gdl/pull/1703/files#diff-3ece204685d8c52e416446a121e65c094e3a5000e452183858d6b07174f9c0a5 (278) A src/shm.hpp https://github.com/gnudatalanguage/gdl/pull/1703/files#diff-625aad1e4578814038d731fb7f1033349807443783fde3b80a5f3838c5b2dba9 (33) Patch Links:

https://github.com/gnudatalanguage/gdl/pull/1703.patch https://github.com/gnudatalanguage/gdl/pull/1703.diff — Reply to this email directly, view it on GitHub https://github.com/gnudatalanguage/gdl/pull/1703, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOC5K6FZITLTA5MTR2VXHU3YMBRA3AVCNFSM6AAAAABBHWI5QSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA3DAOBTGA2DEMI. You are receiving this because you are subscribed to this thread.

brandy125 commented 10 months ago

Thanks @GillesDuvert !!! I tested it and it works as expected! Very nice!

alaingdl commented 10 months ago

@GillesDuvert wouhaouh

I cannot compile on Debian due to relic from the saverestore redefinition of xdr_u_int32_t (unclear for my why this remain ...)

I cannot link on OSX :

[100%] Linking CXX executable gdl
ld: library not found for -lrt
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

But I can compile & test on u2204 ! And the demo provided by @brandy125 is working find for /long and /double ! (but I succeed to crash by after to another local variable the same global one)

codecov[bot] commented 10 months ago

Codecov Report

Attention: 209 lines in your changes are missing coverage. Please review.

Comparison is base (807d262) 42.01% compared to head (e027133) 41.92%. Report is 3 commits behind head on master.

Files Patch % Lines
src/shm.cpp 0.00% 201 Missing :warning:
src/gdlhelp.cpp 37.50% 5 Missing :warning:
src/basegdl.cpp 50.00% 1 Missing :warning:
src/basegdl.hpp 50.00% 1 Missing :warning:
src/shm.hpp 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1703 +/- ## ========================================== - Coverage 42.01% 41.92% -0.09% ========================================== Files 359 361 +2 Lines 96161 96379 +218 Branches 19747 19817 +70 ========================================== + Hits 40401 40410 +9 - Misses 55760 55969 +209 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

GillesDuvert commented 10 months ago

The support is very minimal and risky: there is no mechanism to avoid collisions, the detaching and destroying of shared areas is unsafe at best. @brandy125 for serious work and for the time being you need to have only one process writing and all the others reading. I suppose it is best to insure this minmal safety first before adding more option such as memory mapping real disk files (works with some data files like FITS).

Incidentally, this is a way to share data with IDL as the memory mapping of simple arrays is the same.

alaingdl commented 10 months ago

On u2204 and Debian 10, I can reproduce most of the problems in Codecov :

GDL> test_finite
munmap_chunk(): invalid pointer
Abandon (core dumped)

GDL> test_size
[...]
free(): invalid pointer
Abandon (core dumped)

GDL> .r test_isa
...
double free or corruption (out)
Abandon (core dumped)
GillesDuvert commented 10 months ago

Indeed, last hack had a flaw. Corrected. There will be a possibility of memory leak for rare cases until I set up a better scheme, but this will need a change in the BaseGDL objects.

alaingdl commented 10 months ago

@GillesDuvert tested with success on x86_64 for Debian10, u2204 and OSX 12.3 BUT on OSX I need to remove "-lrt" in src/CMakeFiles/gdl.dir/link.txt to got the link

no more crash.

Once the small problem on OSX fixed, I vote for a merge !

alaingdl commented 10 months ago

@GillesDuvert Linked perfectly on OSX 12.3 ! Still linking on Debian 12 ;) All tests OK on both :)

It is a GO for me for a 1.0.5 !

GillesDuvert commented 10 months ago

OK, probably quite complete. Windows support should be easy afterwards.