IfcOpenShell / IfcOpenShell

Open source IFC library and geometry engine
GNU Lesser General Public License v3.0
1.86k stars 730 forks source link

Unable to reclaim memory when using API #2471

Open aothms opened 2 years ago

aothms commented 2 years ago

I've encountered an issue with Python actually reclaiming memory from an opened file loaded into memory.

I'm using Python 3.8 with ifcopenshell 0.7.0 from PyPI. A snippet from a function in my case:

ifcopenshell_settings = ifcopenshell.geom.settings()
ifcopenshell_settings.set(ifcopenshell_settings.USE_WORLD_COORDS, True)
ifcopenshell_settings.set(ifcopenshell_settings.USE_BREP_DATA, True)
ifcopenshell_settings.set(ifcopenshell_settings.USE_PYTHON_OPENCASCADE, True)

ifc_file = ifcopenshell.open(ifc_filename)

# this doesn't affect garbage collection
xAxisOrdinate, xAxisAbscissa = ifc_file.by_type("IfcProject")[0].RepresentationContexts[0].TrueNorth.DirectionRatios

site = ifc_file.by_type("IfcSite")[0]

# this line prevents memory reclaim and the ifc_file stays in memory indefinitely
conversion_pset = ifcopenshell.api.run("pset.add_pset", ifc_file, product=site, name="ePSet_MapConversion")

After running this snippet multiple times (e.g. in a loop), I'm seeing a memory leak, where the ifc_file doesn't get unloaded. After some time this crashes the host (OOM).

Maybe there's something I'm overlooking when opening or using the file? After removing the last line from the snippet - the memory leak is gone.

Originally posted by @YaroslavKormushyn in https://github.com/IfcOpenShell/IfcOpenShell/issues/650#issuecomment-1265472998

aothms commented 2 years ago

cc @Moult who did the inst -> file pointer I believe

This is an interesting case. I create the following test for it.

import gc
import sys
import pprint
import weakref
import itertools
import ifcopenshell
import ifcopenshell.api

def test(args):
    do_run_api, do_delete_ref, api = args

    f = ifcopenshell.file()
    if api == 0:
        inst = f.createIfcPerson()
    elif api in (1,2):
        inst = f.createIfcSite()

    r = weakref.ref(f)

    print(*args, "ref", r, sys.getrefcount(f))

    if do_run_api:
        if api == 0:
            ifcopenshell.api.run("attribute.edit_attributes", f, product=inst, attributes={'FamilyName': 'Bimson'})
        elif api == 1:
            ifcopenshell.api.run("pset.add_pset", f, product=inst, name="ePSet_MapConversion")
        elif api == 2:
            pset = f.create_entity(
                "IfcPropertySet",
                **{
                    "GlobalId": ifcopenshell.guid.new(),
                    "OwnerHistory": ifcopenshell.api.run("owner.create_owner_history", f),
                    "Name": "ePSet_MapConversion",
                }
            )
            f.create_entity(
                "IfcRelDefinesByProperties",
                **{
                    "GlobalId": ifcopenshell.guid.new(),
                    "OwnerHistory": ifcopenshell.api.run("owner.create_owner_history", f),
                    "RelatedObjects": [inst],
                    "RelatingPropertyDefinition": pset,
                }
            )

    print(*args, "ref", r, sys.getrefcount(f))

    if do_delete_ref:
        del inst.wrapped_data.file
    del inst

    print(*args, "ref", r, sys.getrefcount(f))

    del f

    print(*args, "ref", r)
    print()

list(map(test, itertools.product((0,1),(0,1),(0,1,2))))

It prints:

0 0 0 ref <weakref at 0x0000021A454E7A90; to 'file' at 0x0000021A454F4BB0> 3
0 0 0 ref <weakref at 0x0000021A454E7A90; to 'file' at 0x0000021A454F4BB0> 3
0 0 0 ref <weakref at 0x0000021A454E7A90; to 'file' at 0x0000021A454F4BB0> 2
0 0 0 ref <weakref at 0x0000021A454E7A90; dead>

0 0 1 ref <weakref at 0x0000021A454E7A90; to 'file' at 0x0000021A47087C40> 3
0 0 1 ref <weakref at 0x0000021A454E7A90; to 'file' at 0x0000021A47087C40> 3
0 0 1 ref <weakref at 0x0000021A454E7A90; to 'file' at 0x0000021A47087C40> 2
0 0 1 ref <weakref at 0x0000021A454E7A90; dead>

0 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 2
0 0 2 ref <weakref at 0x0000021A55CF2220; dead>

0 1 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 1 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 1 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 2
0 1 0 ref <weakref at 0x0000021A55CF2220; dead>

0 1 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 1 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 1 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 2
0 1 1 ref <weakref at 0x0000021A55CF2220; dead>

0 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
0 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 2
0 1 2 ref <weakref at 0x0000021A55CF2220; dead>

1 0 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
1 0 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
1 0 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 2
1 0 0 ref <weakref at 0x0000021A55CF2220; dead>

1 0 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
1 0 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
1 0 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60> 3
1 0 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A472AAD60>

1 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A47087BE0> 3
1 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A47087BE0> 4
1 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A47087BE0> 4
1 0 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A47087BE0>

1 1 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 3
1 1 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 3
1 1 0 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 2
1 1 0 ref <weakref at 0x0000021A55CF2220; dead>

1 1 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 3
1 1 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 3
1 1 1 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 2
1 1 1 ref <weakref at 0x0000021A55CF2220; dead>

1 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 3
1 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 4
1 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0> 3
1 1 2 ref <weakref at 0x0000021A55CF2220; to 'file' at 0x0000021A4731DAC0>

I don't know how familiar you all are with garbage collections and weakrefs, but the idea here is that after deleting file and (especially when deleting also site) that the file would be garbage collected (the weakref object would indicate dead). This is usually also the case except when we have used the API, so this matches @YaroslavKormushyn's issue report.

@Moult where again do you set the instance to point to the file?

It's a bit unclear to me why del site didn't result in clearing up the ref to file but I guess it's simply related to swig dark magic that we don't want to understand.

I guess we can simply add a __del__() function to entity_instance which would clear any file references.

Thoughts?

Moult commented 2 years ago

https://github.com/IfcOpenShell/IfcOpenShell/blob/v0.7.0/src/ifcopenshell-python/ifcopenshell/file.py#L258 and https://github.com/IfcOpenShell/IfcOpenShell/blob/v0.7.0/src/ifcopenshell-python/ifcopenshell/entity_instance.py#L110

Commit https://github.com/IfcOpenShell/IfcOpenShell/commit/6b0a58db7dec353dbf931aee7add92e3d97ef77e#diff-b63197ec4d064fc4387ed3f1b8becdbf26fc1c441afea3a07e701d384134a2f7

Is there anything special to the add_pset API? The simplest API to test is attribute.edit_attributes, or is there any difference in going through ifcopenshell.api.run vs manually doing ifcopenshell.api.attribute.edit_attributes.Usecase().execute()?

aothms commented 2 years ago

Thanks @Moult I've updated the code above. Interestingly it is specific to the add_pset call, because calling edit_attribute doesn't result in the alive file object. Only 1 0 1 (✓ do_run_api, ✗ do_delete_ref, ✓ add_pset) shows the issue.

aothms commented 2 years ago

Could it be related to the owner history handling? Is there some global state there that references the file?

Edit, I guess not, because interestingly the refcount doesn't actually increase.

Moult commented 2 years ago

Owner history shouldn't do anything on an IFC4 model with no users or apps.

What about:

ifcopenshell.api.pset.add_pset.Usecase(f, product=inst, name="ePSet_MapConversion").execute()

This will eliminate something funky with the run() wrapper.

Moult commented 2 years ago

Two other things worth investigating is:

ifcopenshell.api.owner.settings.get_user = lambda x : None
ifcopenshell.api.owner.settings.get_application = lambda x : None

... just to be on the safe side, probably won't do much though.

Also try commenting out the return statements return pset in the add_pset API?

aothms commented 2 years ago

Nah, we're on the wrong track. It's not at all related to the API. It seems the issue simply comes from referencing inst in another instance (the relationship) this can be seen when simply inlining the api's code (updated above).

I think I'm just going to go ahead with adding this to entity_instance, unless somebody comes up with a better solution. On the one hand I dislike fully understanding the situation, on the other hand it doesn't read as something exceptionally ugly.

def __del__(self):
    self.wrapped_data.file = None
aothms commented 2 years ago

@YaroslavKormushyn so in summary things should work now with the latest changes, but keep in mind that any live instance you have from a file will prevent the file from getting gc'ed.

aothms commented 2 years ago

Edit: one more general remark. We take memory safety seriously. Still, I would advise against using long-running python processes that import and use complex large modules (such as ifcopenshell or opencascade). Until we're done reimplementing everything in rust, memory leaks (of the smaller kind at least) will probably remain a fact of life. That's also why web-servers such as apache and gunicorn have settings such as MaxRequestsPerChild and max_requests.

YaroslavKormushyn commented 2 years ago

@YaroslavKormushyn so in summary things should work now with the latest changes, but keep in mind that any live instance you have from a file will prevent the file from getting gc'ed.

I'm using the PyPI release of ifcopenshell, so probably without an explicit publish to the package registry this fix won't be usable for me 😅 Could you advise on when this will be published?

Moult commented 2 years ago

@YaroslavKormushyn there's a little delay right now because I've been trying to push to ifcopenshell and for some reason pypi is blocking that name. https://github.com/IfcOpenShell/IfcOpenShell/issues/2468

@YaroslavKormushyn if you want I can give you a wheel though (or you can make your own?) which OS are you on?

YaroslavKormushyn commented 2 years ago

@Moult I'm testing on Windows, production is on Linux.

aothms commented 2 years ago

New IfcOpenBot builds are online https://github.com/IfcOpenBot/IfcOpenShell/commit/fa6bbf2d4a69cb8d1f793b3d4ca86571e9b414e8#commitcomment-85882195

Krande commented 2 years ago

Hey,

not sure it's related but I am seeing an unusual exit code when testing some python scripts using the latest daily build from conda on a Win11 machine. The conda package was compiled ~11 hours ago.

Everything runs fine (all steps are completed during execution), however, I am seeing an unusual exit code -> exit code -1073740940 (0xC0000374) upon termination. I tried this hack, but it did not change anything.

Any chance it could be related to any of the recent changes to the c++ codebase? (like https://github.com/IfcOpenShell/IfcOpenShell/commit/698c708bf6c15ed63918220e47582d1d937c6e5f ?)

aothms commented 2 years ago

Definitely. There were some fixes needed, but after https://github.com/IfcOpenShell/IfcOpenShell/commit/f68db3d3bee94892a8789363400c8cb3df737e1d all the tests here were working again.

At this moment a couple of opening related tests break as @Moult is working on that.

So before we dive into this. Are you sure your recent conda build includes f68db3d3bee94892a8789363400c8cb3df737e1d ?

FLming commented 1 year ago

Sorry, the above content does solve certain problems, but I still encountered OOM, I don't have much experience with the memory monitoring part, I also tried to check why this problem occurred, I used objgraph to check the state of the program running, and found that the number of entity_instance was unusually large, when I had tens of thousands of walls, the entity_instance would reach tens of millions, I don't know if this is the cause of OOM. I checked these tens of millions of entity_instance, all of which were duplicate objects, but each with different IDs.

Here's a code snippet that shows what the code on my product does

import ifcopenshell
import ifcopenshell.api
import objgraph

file = ifcopenshell.file()

objgraph.show_growth()
storey = ifcopenshell.api.run(
    "root.create_entity", file, ifc_class="IfcBuildingStorey", name="Ground Floor"
)
objgraph.show_growth()

for i in range(10):
    print("-" * 80)
    print(f"#{i}".center(80))
    wall = ifcopenshell.api.run("root.create_entity", file, ifc_class="IfcWall")
    print("after create wall")
    objgraph.show_growth()

    ifcopenshell.api.run(
        "spatial.assign_container", file, relating_structure=storey, product=wall
    )
    print("after assign container")
    objgraph.show_growth()
    print(f"#{i}".center(80))
    print("-" * 80)

result is

--------------------------------------------------------------------------------
                                       #0                                       
after create wall
dict                3311        +2
entity_instance        4        +2
after assign container
dict                  3319        +8
function              7944        +6
tuple                 2727        +6
module                 335        +3
ModuleSpec             330        +3
SourceFileLoader       262        +3
getset_descriptor     1185        +2
list                 11194        +2
set                    166        +2
weakref               1454        +1
                                       #0                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #1                                       
after create wall
entity_instance        5        +1
after assign container
dict                  3322        +3
getset_descriptor     1187        +2
function              7946        +2
weakref               1455        +1
module                 336        +1
type                   773        +1
ModuleSpec             331        +1
SourceFileLoader       263        +1
entity_instance          6        +1
                                       #1                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #2                                       
after create wall
entity_instance        7        +1
after assign container
entity_instance        9        +2
                                       #2                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #3                                       
after create wall
entity_instance       10        +1
after assign container
entity_instance       13        +3
                                       #3                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #4                                       
after create wall
entity_instance       14        +1
after assign container
entity_instance       18        +4
                                       #4                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #5                                       
after create wall
entity_instance       19        +1
after assign container
entity_instance       24        +5
                                       #5                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #6                                       
after create wall
entity_instance       25        +1
after assign container
entity_instance       31        +6
                                       #6                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #7                                       
after create wall
entity_instance       32        +1
after assign container
entity_instance       39        +7
                                       #7                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #8                                       
after create wall
entity_instance       40        +1
after assign container
entity_instance       48        +8
                                       #8                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #9                                       
after create wall
entity_instance       49        +1
after assign container
entity_instance       58        +9
                                       #9                                       
--------------------------------------------------------------------------------

In my understanding, ifcwall is a entity_instance, the first time it is allocated to storey because it will add an IFCRELCONTAINEDINSPATIALSTRUCTURE, entity_instance will increase, and then each additional ifcwall will increase entity_instance, and the allocation of storey should not increase.

It looks like every assign creates a copy of all walls.

For contrast, using create_entity does not produce such a phenomenon

import ifcopenshell
import ifcopenshell.api
import objgraph

file = ifcopenshell.file()

objgraph.show_growth()
storey = ifcopenshell.api.run(
    "root.create_entity", file, ifc_class="IfcBuildingStorey", name="Ground Floor"
)
objgraph.show_growth()

for i in range(10):
    print("-" * 80)
    print(f"#{i}".center(80))
    wall = ifcopenshell.api.run("root.create_entity", file, ifc_class="IfcWall")
    print("after create wall")
    objgraph.show_growth()

    # ifcopenshell.api.run(
    #     "spatial.assign_container", file, relating_structure=storey, product=wall
    # )
    file.create_entity("IFCRELCONTAINEDINSPATIALSTRUCTURE", RelatedElements=[wall], RelatingStructure=storey)
    print("after assign container")
    objgraph.show_growth()
    print(f"#{i}".center(80))
    print("-" * 80)

file.write("test.ifc")
--------------------------------------------------------------------------------
                                       #0                                       
after create wall
dict                3311        +2
entity_instance        4        +2
after assign container
                                       #0                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #1                                       
after create wall
entity_instance        5        +1
after assign container
                                       #1                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #2                                       
after create wall
entity_instance        6        +1
after assign container
                                       #2                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #3                                       
after create wall
entity_instance        7        +1
after assign container
                                       #3                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #4                                       
after create wall
entity_instance        8        +1
after assign container
                                       #4                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #5                                       
after create wall
entity_instance        9        +1
after assign container
                                       #5                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #6                                       
after create wall
entity_instance       10        +1
after assign container
                                       #6                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #7                                       
after create wall
entity_instance       11        +1
after assign container
                                       #7                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #8                                       
after create wall
entity_instance       12        +1
after assign container
                                       #8                                       
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
                                       #9                                       
after create wall
entity_instance       13        +1
after assign container
                                       #9                                       
--------------------------------------------------------------------------------
FLming commented 1 year ago

New to the discovery, entity_instance.getattr creates new objects, which also leads to new objects when using constructors such as list(https://github.com/IfcOpenShell/IfcOpenShell/blob/cbd9c898c37bb44500bf3be39c87241cfd8a4fe1/src/ifcopenshell-python/ifcopenshell/api/spatial/assign_container.py#L57). If references to the original object are maintained elsewhere (such as the file object), both the new object and the original object will exist, which may be the reason for the increase in the number of references to the file object.

FLming commented 1 year ago

@Moult I am not very familiar with the underlying design of this repo, if the above description is the source of the problem, please point to the direction of the modification, so that I can contribute to the modification more conveniently.

Moult commented 1 year ago

We cast to list so that we can append a new item there. If the list casting causes something happen to memory management, this is unfortunately something I don't understand - @aothms is the guru here :)

aothms commented 1 year ago

Thank you for these detailed investigations. Hereby some background. Hope we can find a root cause together.

So first of all, we have to keep in mind that there is nothing magical in SWIG (the C++ -> python interface generator) to return identical (meaning same id() in python) for identical objects in C++.

>>> import ifcopenshell
>>> f = ifcopenshell.open('d.ifc')
>>> f[1]
#1=IfcOwnerHistory(#18091,#8812,$,.NOCHANGE.,$,$,$,0)
>>>
>>> a = f[1]
>>> b = f[1]
>>>
>>> id(a), id(b)
(1977979105728, 1977945033936)
>>> id(a.wrapped_data), id(b.wrapped_data)
(1977944960736, 1977944154368)
>>> id(a.wrapped_data.this), id(b.wrapped_data.this)
(1977944960768, 1977979172112)
>>> a.wrapped_data.this
<Swig Object of type 'IfcUtil::IfcBaseClass *' at 0x000001CC86B4EF00>
>>> b.wrapped_data.this
<Swig Object of type 'IfcUtil::IfcBaseClass *' at 0x000001CC88BEF510>

Therefore

>>> a == b
True
>>> a is b
False

Combined with the indirection of ifcopenshell.entity_instance > ifcopenshell.ifcopenshell_wrapper.entity_instance > SwigPyObject it gets extra confusing.

The quick thing to assert though is that at least the most trivial case doesn't leak:

>>> objgraph.show_growth()
...
>>> a = f[1]
>>> del a
>>>
>>> objgraph.show_growth()
>>>

In addition I think it would also be good to run these kind of commands with the necessary del statements (like I did above and maybe gc.collect()) so that we're sure we're not just looking at variables in our scope.

both the new object and the original object will exist, which may be the reason for the increase in the number of references to the file object.

Instanced indeed have a reference to their file in python, but that's only a convencience to bypass gc on the file object so that the file stays alive as long as you have an active reference to an instance of the file.

The other way around, from file to instance, is not available in pure python and is retrieved only from C++.

So, I hope we can work towards a minimal sample with every activate variable del'ed that still leaks.

FLming commented 1 year ago

entity_instance creates a entity_instance when accessing properties, for this example list(contains_elements[0]. RelatedElements), there is a wall 1 in contains_elements[0]. RelatedElements[0], the file maintains a reference to it when the wall is created, and when the property is accessed, we re-create the wall 2(same to wall 1 but with diff id) In the getattr function, whether the file has any position to capture the reference of wall 2 causes a circular reference, resulting in the entity_instance created with the getattr cannot be released, at the same time, the file object cannot be deleted normally. Of course, the above is my guess from the results, because it's not clear to me that self.wrapped_data.add(e.wrapped_data, eid) in the file object will maintain a reference to wall 2.

FLming commented 1 year ago
import ifcopenshell
import ifcopenshell.api
import objgraph

file = ifcopenshell.file()

storey = ifcopenshell.api.run(
    "root.create_entity", file, ifc_class="IfcBuildingStorey", name="Ground Floor"
)

wall = ifcopenshell.api.run("root.create_entity", file, ifc_class="IfcWall")
contained_in_structure = file.create_entity(
    "IFCRELCONTAINEDINSPATIALSTRUCTURE",
    RelatedElements=[wall],
    RelatingStructure=storey,
)

for i in range(10):
    print(f"#{i}".center(80, "-"))
    objgraph.show_growth()
    a = file[1]

for i in range(10):
    print(f"#{i}".center(80, "-"))
    objgraph.show_growth()
    related_elements = list(contained_in_structure.RelatedElements)
    related_elements.append(wall)
    contained_in_structure.RelatedElements = related_elements

I've added the example you used, and entity_instance create from the file object will be freed correctly. here is the output:

---------------------------------------#0---------------------------------------
list                          11192    +11192
function                       7938     +7938
dict                           3313     +3313
tuple                          2735     +2735
builtin_function_or_method     2241     +2241
wrapper_descriptor             2172     +2172
cell                           1534     +1534
weakref                        1453     +1453
method_descriptor              1279     +1279
getset_descriptor              1183     +1183
---------------------------------------#1---------------------------------------
dict                3315        +2
entity_instance        8        +2
---------------------------------------#2---------------------------------------
---------------------------------------#3---------------------------------------
---------------------------------------#4---------------------------------------
---------------------------------------#5---------------------------------------
---------------------------------------#6---------------------------------------
---------------------------------------#7---------------------------------------
---------------------------------------#8---------------------------------------
---------------------------------------#9---------------------------------------
---------------------------------------#0---------------------------------------
---------------------------------------#1---------------------------------------
dict                3317        +2
entity_instance       10        +2
list               11193        +1
---------------------------------------#2---------------------------------------
entity_instance       13        +3
dict                3319        +2
---------------------------------------#3---------------------------------------
entity_instance       17        +4
dict                3321        +2
---------------------------------------#4---------------------------------------
entity_instance       22        +5
dict                3323        +2
---------------------------------------#5---------------------------------------
entity_instance       28        +6
dict                3325        +2
---------------------------------------#6---------------------------------------
entity_instance       35        +7
dict                3327        +2
---------------------------------------#7---------------------------------------
entity_instance       43        +8
dict                3329        +2
---------------------------------------#8---------------------------------------
entity_instance       52        +9
dict                3331        +2
---------------------------------------#9---------------------------------------
entity_instance       62       +10
dict                3333        +2

entity_instance objects are growing at a high rate, as assumed. Theoretically, every time a loop related_elements be covered by a new list, the entity_instance in it should be released, right? @Moult @aothms

aothms commented 1 year ago

Thanks, this is really good info. I also slightly modified your example:

import ifcopenshell
import ifcopenshell.api

import gc
import objgraph

file = ifcopenshell.file()

storey = ifcopenshell.api.run(
    "root.create_entity", file, ifc_class="IfcBuildingStorey", name="Ground Floor"
)

wall = ifcopenshell.api.run("root.create_entity", file, ifc_class="IfcWall")
contained_in_structure = file.create_entity(
    "IFCRELCONTAINEDINSPATIALSTRUCTURE",
    RelatedElements=[wall],
    RelatingStructure=storey,
)

for i in range(1000):
    print(f"#{i}".center(80, "-"))
    objgraph.show_growth()

    related_elements = list(contained_in_structure.RelatedElements)
    related_elements.append(wall)
    contained_in_structure.RelatedElements = related_elements
    del related_elements

    gc.collect()
    objgraph.show_growth()

    print("type:", type(objgraph.by_type('entity_instance')[-1]))
    print("refs:", gc.get_referrers(objgraph.by_type('entity_instance')[-1]))

So with the del related_elements, the symptom is a lot more pure now. And interestingly it is exactly as you say related to the list as the increase of number of instances is equal to the loop iteration.

entity_instance        7        +1
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []
---------------------------------------#1---------------------------------------
entity_instance        9        +2
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []
---------------------------------------#2---------------------------------------
entity_instance       12        +3
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []
---------------------------------------#3---------------------------------------
entity_instance       16        +4
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []
---------------------------------------#4---------------------------------------
entity_instance       21        +5
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []
---------------------------------------#5---------------------------------------
entity_instance       27        +6
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []
---------------------------------------#6---------------------------------------
entity_instance       34        +7
type: <class 'ifcopenshell.ifcopenshell_wrapper.entity_instance'>
refs: []

With the list variable properly deleted, there are no referrers though any more to this object of type ifcopenshell.ifcopenshell_wrapper.entity_instance.

That probably means we're dealing with something wrong with SWIG.

Simplifying the problem further:

>>> import objgraph, ifcopenshell
>>>
>>> f = ifcopenshell.file()
>>> A = f.createIfcPersonAndOrganization()
>>> objgraph.growth()
...
>>> A.ThePerson = f.createIfcPerson()
>>> len(objgraph.growth())
0
>>> B = f.createIfcPolyline()
>>> objgraph.growth()
...
>>> B.Points = [f.createIfcCartesianPoint()]
>>> len(objgraph.growth())
1

^ this is what's happening. If you're setting an attribute of type list of instance, you end-up with N leaked instances, N being the length of the list. This doesn't happen when you get the attribute.

Knowing this it was pretty easy where to look. And indeed on the C side of the extension we use PySequence_GetItem() rather carelessly without decreasing reference count as being done here at the bottom of the page https://pl.python.org/docs/api/refcountDetails.html

I think I made good progress on this in the above commit. @FLming to test, do you compile yourself or how do you install ifopsh?

FLming commented 1 year ago

today, I test it with ifcopenshell-python-38-v0.7.0-fa6bbf2-macos64 from doc page, and this problem happened with my company pc with win10, ifcopenshell download from official page and pypi(code show 10/04?), e508fb4 and fa6bbf2 all came up with OOM。

FLming commented 1 year ago

I'll look at how to compile the project later to see if the above commit fixes the problem. BTW, del function in entity_instance, is it necessary?

aothms commented 1 year ago

I'll start a new build.

For del function see:

https://github.com/IfcOpenShell/IfcOpenShell/issues/2471#issuecomment-1266511887

aothms commented 1 year ago

Edit: oops, builds are broken currently, need to some time to look into that..

FLming commented 1 year ago

I'll start a new build.

For del function see:

#2471 (comment)

References to file object, now it seems that those leaking objects are caused? isn't it? Also explains why 1 0 0 is normal.

Sorry, it doesn't seem to be the same issue

aothms commented 1 year ago

New builds are up https://github.com/IfcOpenBot/IfcOpenShell/commit/1d6c3b3fc2ee60163b5944b89440f34ac80958ed#comments