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

add reallocMemory to callback functions #122

Closed
modelica-trac-importer opened this issue Oct 16, 2018 · 15 comments
Closed

add reallocMemory to callback functions #122

modelica-trac-importer opened this issue Oct 16, 2018 · 15 comments
Assignees
Labels
needs poll This issue needs polling to proceed
Milestone

Comments

@modelica-trac-importer
Copy link

Reported by andreas.junghanns on 4 Jan 2013 16:14 UTC
Currently, the callbacks specified for memory management are allocate and free.

If FMU code uses realloc() then this cannot be faked by malloc-memcpy-free because the previous size of the memory is missing for the memcpy action.

Suggestion: Replace allocateMemory with reallocateMemory and accept a first argument that can be NULL.

[email protected] requested such a function as well in his email from Feb 16, 2012.


Migrated-From: https://trac.fmi-standard.org/ticket/122

@modelica-trac-importer
Copy link
Author

Comment by iakovn on 8 Jan 2013 22:59 UTC
I got a negative response on my mail mentioned in the ticket and investigated the issue a bit deeper. There are some strong reasons why realloc is prohibited in many systems. For instance, Steve Maguire's Writing Solid Code explains why realloc is horrible.

The real problem is it does so much, and any combination of parameters is valid; you can pass zero and non-zero in any combination.

It does: malloc, free, shrink and relocate, where shrink would make an allocated block smaller and relocate would move to a new block.

His conclusion was that it was such a common source of error (because it is misunderstood) that it shouldn't be used directly. And I have to say, I agree.

There is no realloc in C++ neither (if one uses only new and delete).
With this arguments I can understand why this callback in not present in the standard interface. Luckily, most numerical codes do not need realloc anyway.
If one needs to "fake" realloc then every malloc request may be extended with a header containing information on the memory block size before calling allocateMemory. This information may then be used to perform memcpy in the realloc.

To make a decision on the standard it might be worth too make a poll on which importing environments do not provide "native" realloc vs which FMUs need the functionality. Note that there is even a capability flag "canNotUseMemoryManagementFunctions".

@modelica-trac-importer
Copy link
Author

Modified by rerbacher on 6 Feb 2013 11:07 UTC

@modelica-trac-importer
Copy link
Author

Comment by torstenblochwitz on 7 Feb 2013 14:55 UTC
Decision: Follow the last remarks from Jakov: realloc will not be included.

@modelica-trac-importer modelica-trac-importer added the wontfix This will not be worked on label Oct 16, 2018
@modelica-trac-importer
Copy link
Author

Comment by karl.wernersson on 26 Jun 2018 13:56 UTC
Requested by array working group, should be reopened again for 3.0

@modelica-trac-importer modelica-trac-importer removed the wontfix This will not be worked on label Oct 16, 2018
@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 6 Jul 2018 12:42 UTC
I believe that the statements quoted above from Steve Maguire's Writing Solid Code are still valid.

One of the issues Steve Maguire lists is the mix of error code and return value - and the obvious solution he proposes is changing the interface to:

flag fResizeMemory(void**ppv, size_t sizeNew);

(The idea is that if the resize fails ppv will keep its old value - and that memory block will still be valid; so you can handle errors without the associated risk of having an invalid pointer.)

If we extend this to have the old size as input as well (instead of getting it from the memory allocator) we get

flag fResizeMemory(void**ppv, size_t sizeNew, size_t sizeOld);

which could be implemented inside FMUs using the currently available functions (by doing nothing if same size and otherwise always allocating a new block and copying).
I assume the sizeOld argument would be trivial to handle when re-allocating (if you use it to allocate it is just zero).

That means we have an proposed interface that can already be used and implemented with existing functions - inside an FMU without changing the specification.

This simplifies testing of this proposed change - we can see how/whether realloc really makes these FMUs faster compared to the existing functions, and the cost of zeroing the memory - and we could even reallocate and guarantee that newly allocated memory is NUL. The proposed interface also makes it easy to implement reallocation-function in languages where no realloc is available (I also looked at #435), in case we decide to actually provide reallocation-function.

Addendum: Unless testing reveals that realloc is significantly faster for FMUs I don't see any reason to have a reallocation-function in the standard.

@modelica-trac-importer
Copy link
Author

Comment by anonymous on 25 Jul 2018 14:00 UTC
EDF don't see any reason to have a reallocation-function in the standard. This will complicate the interface.

@modelica-trac-importer
Copy link
Author

Comment by pierre.mai on 1 Aug 2018 15:04 UTC
In aid of the ongoing discussion on the future of the memory management callbacks, here are some numbers on the Test_FMUs currently available in the cross-check repos: The following tables show the number of FMUs that declare that they cannot use the callbacks (canNotUseMemoryManagementFunctions="true"), those that explicitly declare that they can use them (canNotUseMemoryManagementFunctions="false"), and those which have no such attribute, which defaults to false (however it is not unlikely in those cases that some FMUS do not declare due to oversight, but still use non-callback memory allocation functions).

FMI 2.0

= Description = = Occurrences =
= canNotUse Flag true = 173
= canNotUse Flag false = 103
= No flag, i.e. false = 506
= Total FMUs = 782

FMI 1.0

= ** Description ** = = Occurrences =
= canNotUse Flag true = 114
= canNotUse Flag false = 59
= No flag, i.e. false = 683
= Total FMUs = 856

As said above, it is not unlikely that a number of FMUs which do not declare that they cannot use the memory management callbacks do in fact still use memory management functionality of the OS without declaring this (since there is no obvious way for testing this).

@TorstenBlochwitz
Copy link

Discussion at the FMI Design Meeting in Roanne:

  1. Remove mem alloc callbacks allocateMemory and freeMemory (6)
  2. Fix mem alloc functions (9)
  3. Keep mem alloc callbacks allocateMemory and freeMemory (5)

Second poll:

  1. Replace allocateMemory by reallocMemory (1)
  2. Replace allocateMemory by recallocMemory (2)
  3. Add reallocMemory but change allocateMemory to behave as malloc (6)
  4. Add reallocMemory (10)
  5. Add recallocMemory (3)

@chrbertsch
Copy link
Collaborator

Is there still the intention to fix this in FMI 3.0? Introducing a reallocMemory callback function?
If yes, someone has to create a PR, otherwise it will not make it in FMI 3.0.
@iakovn, @andreas-junghanns, @pmai , @klausschuch and others: is anyone volunteering?

@andreas-junghanns andreas-junghanns modified the milestones: v3.0, v3.1, v4.0 Feb 28, 2020
@andreas-junghanns
Copy link
Contributor

I think this should be addressed when we deal with the lifetime issues and that will have to wait, right?
We now know that we cannot do that in 3.1, so I set the milestone to 4.0.
If anyone thinks there is a chance to do this in 3.0, please submit a PR.

@chrbertsch
Copy link
Collaborator

I think this should be addressed when we deal with the lifetime issues and that will have to wait, right?

@andreas-junghanns : you mean #515, right ?

We now know that we cannot do that in 3.1, so I set the milestone to 4.0.

Well, #511 has target milestone 3.1. I understand that @t-sommer and @pmai consider this possible, right?

@chrbertsch chrbertsch modified the milestones: v4.0, v3.1 Feb 29, 2020
@andreas-junghanns
Copy link
Contributor

With inlining the structures into the function calls (#821) adding a new function requires new function signatures. Theoretically, we could do that using an additional (alternative?) function, but in our 3.0 discussions, we seemed to avoid this kind of solution. This is why I thought I move this to 4.0. This can of course be changed if a discussion results in us changing our mind.

@andreas-junghanns
Copy link
Contributor

Discussion at April 2020 Design Meeting:
TorstenS: What is the use case for these memory functions? Can´t we remove these callback functions?
ChristianS: We used this for checking allocation and free.
AndreasJ: This is debugging - aren´t there better ways?
Markus: For debugging, we have a lot of other ways to chase these issues.
ChristianS: We can use this to debug on customer PCs. Or we can map memory of kernel memory.
TorstenS: Why don´t we remove them?
TorstenS: If the importer can use memory management, why not the FMU itself? The platform is clear: the binary was compiled for it.
AndeasJ: I have seen this for efficiency reasons: Importers can bypass the system memory allocation if needed. But when that is required, would the FMU exporter not know?
TorstenS: The only reason to do better memory handling because you know what the FMU needs, the OS cannot know and must be generic and therefore is slower - this invalidates the efficiency argument.
Pierre: We should drop the memory handling callbacks. They do not buy anything and do not guarantee to solve any issue.
AndreasJ: I agree with this.
Pierre: this starts to provide a common runtime for FMUs. But this is not complete, it misses file access, synchronization, timing... the only area where a common minimal runtime would make sense is a common portable binary FMU (intermediate code).
AndreasJ: Than we would use one of the other solutions out there (JVM or whatever) and build stuff on top of this.
AndreasJ: We can ask if there is a use case for these memory callbacks.
AndreasJ: Let´s put this up for vote in the larger group at 14:00.

@andreas-junghanns andreas-junghanns added April2020 needs poll This issue needs polling to proceed and removed bug Something isn't working discussion labels Apr 22, 2020
@andreas-junghanns andreas-junghanns modified the milestones: v3.1, v3.0 Apr 22, 2020
t-sommer added a commit to t-sommer/fmi-standard that referenced this issue Apr 22, 2020
- fmi3CallbackAllocateMemory
- fmi3CallbackFreeMemory

and attribute "canNotUseMemoryManagementFunctions"

see modelica#122
@andreas-junghanns
Copy link
Contributor

Poll again April 2020 FMI Design Meeting.
General consensus: Malloc and Free create more issues than they solve, most of the benefits are minor or theoretical and broken most of the time.

  1. Remove mem alloc callbacks allocateMemory and freeMemory: Andreas, Antoine, Claudio, Klaus, Markus, Otto, Pierre, Robert, Torsten (9)
  2. Fix mem alloc functions (add instance pointer): None (0)
  3. Keep mem alloc, free callbacks and add reallocateMemory: None (0)
  4. Abstain: Irina, Karl (2)

@andreas-junghanns
Copy link
Contributor

PR #928 was created and will be merged.

chrbertsch added a commit to boschresearch/fmi-standard that referenced this issue Dec 10, 2020
* Create FMI_Project_news.md

* Update according to review

* Changes according to review of Thomas Beutlich

* Changes proposed by Torsten Blochwitz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs poll This issue needs polling to proceed
Projects
None yet
Development

No branches or pull requests

6 participants