Skip to content

lazy loaded clib#208

Merged
wonder-sk merged 16 commits intomasterfrom
shutdown
Feb 5, 2026
Merged

lazy loaded clib#208
wonder-sk merged 16 commits intomasterfrom
shutdown

Conversation

@PeterPetrik
Copy link
Contributor

@PeterPetrik PeterPetrik commented Jan 16, 2024

  • introduced pygeodiff.shutdown() to correctly unload on windows
  • pulled out context from geodifflib to geodiff
  • geodifflib is now shared between all instances of pygeodiff.GeoDiff()

latest python had some problems with comments with \ so I replaced it with :

fix #205

@PeterPetrik PeterPetrik changed the title WIP: lazy loaded clib lazy loaded clib Jan 17, 2024
@dvdkon
Copy link
Contributor

dvdkon commented Dec 19, 2025

I rebased this PR on current master.

@wonder-sk
Copy link
Contributor

actually...

  • is it correct that GeoDiff class calls shutdown() upon its destruction? I think that may cause a series of loads and unloads of the geodiff C library when GeoDiff is created and deleted in sequence - or not?
  • and do I understand it correctly that each GeoDiff has its own GeoDiffLib instance? (the PR description says the geodifflib should be shared now?)

@dvdkon
Copy link
Contributor

dvdkon commented Dec 19, 2025

  • is it correct that GeoDiff class calls shutdown() upon its destruction? I think that may cause a series of loads and unloads of the geodiff C library when GeoDiff is created and deleted in sequence - or not?

Hm, I missed that. I think it doesn't make sense, since GeoDiffLib.shutdown() is also called by GeoDiffLib.__del__. But loading/unloading the library when multiple GeoDiff objects are created with distinct lifetimes is probably unavoidable, if the goal is to unload the library when it's no longer needed.

  • and do I understand it correctly that each GeoDiff has its own GeoDiffLib instance? (the PR description says the geodifflib should be shared now?)

Yes, this way we'll end up with multiple GeoDiffLib instances (that will clean up after themselves on Windows, but still). I'll commit a fix.

@wonder-sk wonder-sk merged commit 4cd6091 into master Feb 5, 2026
26 checks passed
def __del__(self):
self.shutdown()

def _lazy_load():
Copy link
Contributor

@volcan01010 volcan01010 Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An unsolicited comment from me.

You could use Python's @property decorator here to simplify the lazy loading.

https://realpython.com/python-property/

It would look something like this.

def __init__(self, libname=None):
    ...
    self._clib = None
    self._context = None

@property
def clib(self):
    if self._clib is None:
        self._clib = GeoDiffLib(self.libname)
    return self._clib

@property
def context(self):
    if self._context is None:
        self._context = self.clib.create_context()
        context.callbackLogger = None
    return self._context

def shutdown(self):
    self.clib.destroy_context(self.context)
    self._clib = None
    self._context = None

This means that you don't have to call _lazy_load all the time, as the propery creates the clib and context when they are first accessed. It does mean that the shutdown function will create new clib and context just to destroy them if you call it before you have used them elsewhere.

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.

Add a method to unload geodiff library from python

5 participants