Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shmmap etc #1703

Merged
merged 20 commits into from
Jan 4, 2024
Merged

Shmmap etc #1703

merged 20 commits into from
Jan 4, 2024

Conversation

GillesDuvert
Copy link
Contributor

WIP but can be tested

@brandy125
Copy link

brandy125 commented Dec 30, 2023 via email

@brandy125
Copy link

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

@alaingdl
Copy link
Contributor

@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)

Copy link

codecov bot commented Dec 31, 2023

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 ⚠️
src/gdlhelp.cpp 37.50% 5 Missing ⚠️
src/basegdl.cpp 50.00% 1 Missing ⚠️
src/basegdl.hpp 50.00% 1 Missing ⚠️
src/shm.hpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GillesDuvert
Copy link
Contributor Author

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.

…mapped memory when destroyed.

probably better to replace all that by a simpler variant of the Assoc mechanism, but the use of a DPtrGDL here permits to get the memory cleaned when the referenc count drops to zero.
@alaingdl
Copy link
Contributor

alaingdl commented Jan 1, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

alaingdl commented Jan 2, 2024

@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 !

…map list, correct handling of pending delete shared memory and offsets.
@alaingdl
Copy link
Contributor

alaingdl commented Jan 2, 2024

@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
Copy link
Contributor Author

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

@GillesDuvert GillesDuvert merged commit 8e75965 into gnudatalanguage:master Jan 4, 2024
6 of 8 checks passed
@GillesDuvert GillesDuvert deleted the shmmap_etc branch July 2, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants