-
Notifications
You must be signed in to change notification settings - Fork 68
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
Eliminate Leaks by wrapping resources in contextlib with-blocks #36
Conversation
+ fix(loose-db): fix bad-attr in ex-message
+ feat(util): add logger. + feat(util): add suppress-ex context-handler (from PY3 sources).
+ Packers MUST be invoked inside `Withh...` blocks, or `_cursor` won't exist! + Had to drop NotLazy for their hierarchy :-( + Count entrances/exits. + feat(util: add `rmtree()` for READ_ONLY files on Windows. 3-->2 Windows TCs now fail.
+ stop importing git-submodules for gitdb & smmap
b17f91a
to
eaf13b1
Compare
+ File-in-use errors were fixed with `gitdb.util.mman.collect()`! + This call is disabled `gitdb.util.HIDE_WINDOWS_KNOWN_ERRORS == False`. + Depend on latest smmp `v2.1.0.dev1` tag
Speed-up for non-error commits: + no-leak contextlibing(v.2.0.0, 941b6c7): 5.20 + Prev commit, no LazyMixin(v.2.1.0.dev1): 7.70 + This commit, LazyMixin: 5.50
295065c
to
ad3eb79
Compare
873e544
to
2376a24
Compare
+ chrore(deps): depend on *contextlib2* for `ExitStack` in PY2. + refact(util): BREAKING API move consts out of utils. + style(pep8): fixe all sources.
speedups: + 3.5: ~5% faster + 3.4: ~15% faster + 3.3/2.7/2.6: the same BREAKING API 1: `XXOStream` not an instance of `XXOInfo`. BREAKING API 2: `XXOinfo/XXOStream not inheritable. (ie no `DeriveTest` class).
ea64935
to
bc65c8d
Compare
+ chore(deps): + smmap2-v2.1.0.dev4 (FIXes memoryview leak). + FIX quoted environment marker in requirements. + import actually smmap2!
@ankostis Did you delete the branch because gitdb is only optional in GitPython? I am asking because I thought this one was close to being mergeable, given the immense amount of work that went into it. |
Yes that was one of the reasons. You see, this PR still gitdb didn't behave correctly, and then I discovered the memoryview leaks in smmap lib and moved into this project to fix it, and in the process, I had to do massive API-breaking changes. See this personal branch which is a total rewrite based on the conclusions I had layed out in gitpython-developers/smmap/issues/31#issuecomment-256004958. Then I was planning to reimplement this PR based on the new smmap but, 1) i had other priorities, and 2) I knew that would require more work on the GitPython project to re-enact the use of GitDb, and I couldn't see when I would have the time for all this, so I kind-of abandoned it, |
Alright, thanks for the summary :)! That's how it happens sometimes. And having one functional backend is probably sufficient anyway - pure-python (i.e. gitdb) is not really an advantage anyways, just because the cgit executable is available everywhere. |
BREAKING API: retrofit streams and packers as context-managers.
Specifically packers (
git.pack.PackIndexFile
,git.pack.PackFile
&git.pack.PackEntity
)must always be used from within a
with ..
block, or you will get mmap-cursor missing errors... Note::
You can "enter" `PackIndexFile
&
PackFile``multiple time, but``PackEntity``only onceto detect and avoid sloppy deep-nesting.
Since``git.pack.PackEntity``class just coalseces``PackIndexFile``&``PackFile``,
you may "enter" either each internal packer separately, or the entity only once.
BREAKING API: some utilities moved between
git.util
,git.const
&git.utils.compat
.Fix (probably) all leaks in Windows.
.. Note::
The problem is that on Linux, any open files go unoticed, or collected by GC.
But on Windows (and specifically on PY3 where GC is not deterministic),
the underlying files cannot delete due to access violation.
That's a Good-thing©, because it is dangerous to leak memory-mapped handles.
Actually Windows may leak them even after process who created them have died,
needing a full restart(!) to clean them up (signing-out is not enough).
Stop depending on smmap submodule - deleted completely.
Developer now has to specify any specific dependecy to smmap in
requirements.txt
file, andremember to updated before a final release.