Page MenuHomePhabricator

eo: add the ability to get the size of object of a certain class.
ClosedPublic

Authored by cedric on Dec 7 2018, 2:48 PM.

Details

Summary

Their was two different way to implement this, either like this with
a simple function that work on Efl_Class, or by a function on
Efl.Object. I leaned on the first one, but I could easily be convinced
it should be done on Efl.Object actually.

Depends on D7440

Diff Detail

Repository
rEFL core/efl
Branch
T7485-devs/cedric/caching_factory
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8509
cedric created this revision.Dec 7 2018, 2:48 PM
segfaultxavi requested changes to this revision.Dec 8 2018, 12:46 AM

Looks like this method accepts both a class and an object. This should be explained in the doc.

Also, since this adds API, some tests would be nice.

src/lib/eo/Eo.h
939

@param, and start sentece capitalised

This revision now requires changes to proceed.Dec 8 2018, 12:46 AM
cedric updated this revision to Diff 17913.EditedDec 14 2018, 10:24 AM
cedric edited the summary of this revision. (Show Details)

I don't have much idea on how to write a useful test at all here. The size can change with pretty much every release (it can go up and down). The only thing I could test is if it is > 0, would that be useful ?

SanghyeonLee added a comment.EditedDec 16 2018, 10:51 PM

is this object support in parameter for backward compatibility?
if not,
why not we restrict parameter Efl_Class and other cases, user call efl_class_name(memory_size)_get(efl_class_get(obj))?
we could give this usage with some macros
or new API like,
efl_object_class_name_get
efl_object_class_memory_size_get

@SanghyeonLee The reason (at least) for efl_class_name_get to handle a object and class is, that it is super easy to get the name of a object in the debugger, p efl_class_name_get(obj) is easily possible without needing to chain multiple functions. However, a macro would not work in the debugger. I guess its pretty much the same thing here for this function :)

I don't have much idea on how to write a useful test at all here. The size can change with pretty much every release (it can go up and down). The only thing I could test is if it is > 0, would that be useful ?

Hmmm, you are right. Adding a check with a constant size would mean changing the test every time the classes (or objects) change size, which would be annoying, for sure. HOWEVER, this test would warn developers who have increased the class size inadvertently, which might be useful.
I don't know, I'll trust your opinion :)

segfaultxavi accepted this revision.Dec 17 2018, 8:28 AM
This revision is now accepted and ready to land.Dec 17 2018, 8:28 AM

I don't have much idea on how to write a useful test at all here. The size can change with pretty much every release (it can go up and down). The only thing I could test is if it is > 0, would that be useful ?

Hmmm, you are right. Adding a check with a constant size would mean changing the test every time the classes (or objects) change size, which would be annoying, for sure. HOWEVER, this test would warn developers who have increased the class size inadvertently, which might be useful.
I don't know, I'll trust your opinion :)

That's an idea. I can write a test that check that the size is at max the current one. This would reduce the amount of time we have to deal with the test requiring an update and every time someone spend time optimizing EFL for size it could reduce that value just to make sure nobody is going to tractor is work. So will add a test along that line with a description of what is going on and why the test would break.

cedric updated this revision to Diff 18125.Dec 27 2018, 4:05 PM

Rebase.

raster added a subscriber: raster.Dec 28 2018, 2:27 AM

I don't have much idea on how to write a useful test at all here.

I would agree - it's not worth testing. What is the point of getting the size of just the obj_size - this is just the struct. it doesn't include sub-structs that are maybe allocated on the fly and so on... so from a profiling/debugging point of view it's not going to accurately reflect how much memory an object actually uses... ?

I would like to want to know why eo needs 'the ability to get the size of object of a certain class' ?

@kimcinoo - good question because getting size of the class doesn't get the size of every object as i mentioned above - you could store sub-structs, lists (and the list nodes and what they point to) strings etc. so every obj could be a different size. this just gets base absolute minimum size. i'm not so sure it's really a great and useful thing as any old malloc debugger can probably tell you this kind of info (with track trace to the allocation etc.). a proper "get the mem used now of THIS object to the best of its knowledge" is more useful IMHO. it would mean every class has to implement this (track its own allocations and add them up then ask parent class etc.) it'd be a lot of extra work though.

This revision was automatically updated to reflect the committed changes.