#25 Implement memory_usage() builtin function

Closed
opened 3 years ago by belliash · 21 comments
belliash commented 3 years ago
Owner

In scope of this ticket, we need to implement additional builtin function - memory_usage(). It should return a number of bytes allocated by Aer Interpreter.

In scope of this ticket, we need to implement additional builtin function - memory_usage(). It should return a number of bytes allocated by Aer Interpreter.
belliash added the
enhancement
label 3 years ago
belliash changed title from Implement get_memory_usage() builtin function to Implement memory_usage() builtin function 3 years ago
devnexen commented 3 years ago
Collaborator

Is it relevant (and possible) as well the max memory usage ?

Is it relevant (and possible) as well the max memory usage ?
belliash commented 3 years ago
Poster
Owner

Yeah, why not. I think this task can be even bigger. If we need to modify the memory allocation mechanism, to save such information, we can also already check if current memory usage + what we are going to allocate does not exceed some limit set.

Yeah, why not. I think this task can be even bigger. If we need to modify the memory allocation mechanism, to save such information, we can also already check if current memory usage + what we are going to allocate does not exceed some limit set.
devnexen commented 3 years ago
Collaborator

I made a basic version, of the first part only, locally but not sure about the approach.

1/ Storing the info into VM allocator.

2/ Counting occurs within MemBackendAlloc/Realloc (i.e the "old" address)/Free.

3/ But then it s counting the total size of the block rather than just the requested size.

I made a basic version, of the first part only, locally but not sure about the approach. 1/ Storing the info into VM allocator. 2/ Counting occurs within MemBackendAlloc/Realloc (i.e the "old" address)/Free. 3/ But then it s counting the total size of the block rather than just the requested size.
belliash commented 3 years ago
Poster
Owner

Generally speaking, I was thinking about some structure storing all necessary information, like for example:

struct ph7_heap {
    sxi32 size; /* current memory usage */
    sxi32 peak; /* peak memory usage */
    sxi32 limit; /* memory limit */
}

This is some sort of minimal version storing all data in bytes.
SyOSHeapAlloc() and related takes sxu32 nByte argument which tells how much memory should be allocated. This is finally passed to malloc() or realloc() under Unix-family OS. I think we can this value simply add to ph7_heap.size and check if it is bigger than peak. If so, overwrite the peak with current value.
The SyOSHeapRealloc() and SyOSHeapFree() takes a pointer which I think contains information about current size, doesn't it?
Also having the limit set, we can check if size + requested amount of memory does not exceed the limit.

I think I don't understand why it should be counting size of block?

Generally speaking, I was thinking about some structure storing all necessary information, like for example: struct ph7_heap { sxi32 size; /* current memory usage */ sxi32 peak; /* peak memory usage */ sxi32 limit; /* memory limit */ } This is some sort of minimal version storing all data in bytes. SyOSHeapAlloc() and related takes sxu32 nByte argument which tells how much memory should be allocated. This is finally passed to malloc() or realloc() under Unix-family OS. I think we can this value simply add to ph7_heap.size and check if it is bigger than peak. If so, overwrite the peak with current value. The SyOSHeapRealloc() and SyOSHeapFree() takes a pointer which I think contains information about current size, doesn't it? Also having the limit set, we can check if size + requested amount of memory does not exceed the limit. I think I don't understand why it should be counting size of block?
devnexen commented 3 years ago
Collaborator

Yes pointer contains info about the size this is how xChunkSize function pointer count on.
In fact that was my question, counting the real size allocated vs just the bytes requested.

Ok I see your point but maybe the type of the heap struct are a bit small if we count in byte and why signed types ?

Yes pointer contains info about the size this is how xChunkSize function pointer count on. In fact that was my question, counting the real size allocated vs just the bytes requested. Ok I see your point but maybe the type of the heap struct are a bit small if we count in byte and why signed types ?
belliash commented 3 years ago
Poster
Owner

The structure was just a quick example. AFAIK this is how PHP counts the memory, however its zend_mm_heap structure contains more information, like about list of huge blocks, unused chinks, cached chunks, and so on...

The structure was just a quick example. AFAIK this is how PHP counts the memory, however its zend_mm_heap structure contains more information, like about list of huge blocks, unused chinks, cached chunks, and so on...
devnexen commented 3 years ago
Collaborator

Ok I ve made a quick test (did not do the limit part yet)

`
class Main {

    public function __construct() {
            $this->displayMem();
    }

    private function displayMem() {
            $bytes = memory_get_usage();
            $peak = memory_get_peak_usage();
            var_export($peak);
            var_export($bytes);
            var_export($peak >= $bytes);
    }

}

new Main();
`

resulting in my case

283377 283297 TRUE

As a test would display only the latter for the .exp counterpart ...

Basically what I did :

1/ ph7_heap type within SyMemBackend, all zeroed at init already.
2/ Changing the MemOS* signatures to accept this type, updating the peak accordingly too.
3/ I named similarly to php but it does not please, easy to rename.
`

Ok I ve made a quick test (did not do the limit part yet) ` class Main { public function __construct() { $this->displayMem(); } private function displayMem() { $bytes = memory_get_usage(); $peak = memory_get_peak_usage(); var_export($peak); var_export($bytes); var_export($peak >= $bytes); } } new Main(); ` resulting in my case ` 283377 283297 TRUE ` As a test would display only the latter for the .exp counterpart ... Basically what I did : 1/ ph7_heap type within SyMemBackend, all zeroed at init already. 2/ Changing the MemOS* signatures to accept this type, updating the peak accordingly too. 3/ I named similarly to php but it does not please, easy to rename. `
belliash commented 3 years ago
Poster
Owner

I just like to add, that it might be hard to use above code as test. I imagine that memory usage values can be different every script run, and thus test will fail. If we want to test it, we might need to think about some script not showing the integer values.

I just like to add, that it might be hard to use above code as test. I imagine that memory usage values can be different every script run, and thus test will fail. If we want to test it, we might need to think about some script not showing the integer values.
devnexen commented 3 years ago
Collaborator

I know exactly what I said -> showing only the last case :-) the values was only for demo here.

I know exactly what I said -> showing only the last case :-) the values was only for demo here.
belliash commented 3 years ago
Poster
Owner

Do you think we could check if a value returned by both functions are positive integers and simply output true or false? Does such a test make sense?

Do you think we could check if a value returned by both functions are positive integers and simply output true or false? Does such a test make sense?
devnexen commented 3 years ago
Collaborator

You mean > 0 (in my case types are 64 types unsigned) ? Yes we could add these too.

You mean > 0 (in my case types are 64 types unsigned) ? Yes we could add these too.
devnexen commented 3 years ago
Collaborator

A little progress, the memory limit for the moment passed by the command line (did not copy yet the multiplier behavior of php i.e 100M etc)

binary/aer -M100000 tests/memory_usage.aer Out of memory, max allowed 100000 bytes, allocated 90328 bytes

A little progress, the memory limit for the moment passed by the command line (did not copy yet the multiplier behavior of php i.e 100M etc) ` binary/aer -M100000 tests/memory_usage.aer Out of memory, max allowed 100000 bytes, allocated 90328 bytes `
devnexen was assigned by belliash 3 years ago
devnexen commented 3 years ago
Collaborator

Finally got a relatively working version of these, thanks to @belliash for early tests !

Finally got a relatively working version of these, thanks to @belliash for early tests !
belliash commented 3 years ago
Poster
Owner

I moved all work to memory_limit branch.

I moved all work to memory_limit branch.
belliash commented 3 years ago
Poster
Owner

There is still some issues to address. There is a problem with displaying an 'out of memory' error message if some limit is set.

There is still some issues to address. There is a problem with displaying an 'out of memory' error message if some limit is set.
belliash commented 3 years ago
Poster
Owner

I have reviewed this once again and I think there is no need to pass SyMemBackend structure down to MemOS* functions as the calculations can be done in MemBackedn* functions. They already have access to SyMemBackend, what simplifies whole implementation.

I also wonder if we could use pointers, in SyMemBackendInit() and SyMemBackendInitFromParent() to avoid copying the memory usage statistics data (ph7_heap structure)...

I have reviewed this once again and I think there is no need to pass SyMemBackend structure down to MemOS* functions as the calculations can be done in MemBackedn* functions. They already have access to SyMemBackend, what simplifies whole implementation. I also wonder if we could use pointers, in SyMemBackendInit() and SyMemBackendInitFromParent() to avoid copying the memory usage statistics data (ph7_heap structure)...
devnexen commented 3 years ago
Collaborator

I have tried but the values are very inconsistent in this case. Plus not sure the statistics are copied between backends ? Even looking at InitFromParent, only the memory limit is copied.

I have tried but the values are very inconsistent in this case. Plus not sure the statistics are copied between backends ? Even looking at InitFromParent, only the memory limit is copied.
belliash commented 3 years ago
Poster
Owner

This implementation is based on @devnexen work and the attached patch can be applied on top of master branch. I think everything is implemented and working, except there is no SAPI parameter to set the memory limit. Still needs some testing.

@devnexen, looking forward for your review.

This implementation is based on @devnexen work and the attached patch can be applied on top of master branch. I think everything is implemented and working, except there is no SAPI parameter to set the memory limit. Still needs some testing. @devnexen, looking forward for your review.
devnexen commented 3 years ago
Collaborator

The MemBackendCalculate could be a macro to avoid one function call otherwise looks reasonably good for a first stage. Not against calculating the chunk size instead of the fcn. pointer. Also yes needs testing (i.e. realloc data for example).

The MemBackendCalculate could be a macro to avoid one function call otherwise looks reasonably good for a first stage. Not against calculating the chunk size instead of the fcn. pointer. Also yes needs testing (i.e. realloc data for example).
likoski commented 3 years ago
Owner

There is not much difference between function and macro, especially there are several functions called already [SyMemBackendAlloc() -> MemBackendAlloc() -> MemOSAlloc() -> SyOSHeapAlloc()]. Modern compilers can optimize the code well and we should rely on them.

I browsed the attached diff and I noticed that you also call MemBackendCalculate() from SyMemBackendInit() to count the size of SyMemHeap. Does it make a big difference in resulting memory usage?

You both done a good job.

There is not much difference between function and macro, especially there are several functions called already [SyMemBackendAlloc() -> MemBackendAlloc() -> MemOSAlloc() -> SyOSHeapAlloc()]. Modern compilers can optimize the code well and we should rely on them. I browsed the attached diff and I noticed that you also call MemBackendCalculate() from SyMemBackendInit() to count the size of SyMemHeap. Does it make a big difference in resulting memory usage? You both done a good job.
likoski commented 3 years ago
Owner

I have done testing and applied patch seems to be working fine. Also fixed memory_limit branch.

I have done testing and applied patch seems to be working fine. Also fixed memory_limit branch.
belliash closed this issue 3 years ago
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.