8000 Reference leak when comparing structured scalars · Issue #6250 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content
8000

Reference leak when comparing structured scalars #6250

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.

Sign up for GitHub

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

Jump to bottom
Closed
pitrou opened this issue Aug 26, 2015 · 6 comments
Closed

Reference leak when comparing structured scalars #6250

pitrou opened this issue Aug 26, 2015 · 6 comments

Comments

@pitrou
Copy link
Member
pitrou commented Aug 26, 2015

If you execute the following code, you'll it leaks a reference to the array:

recordtype = np.dtype([('a', np.float64), ('b', np.int32), ('d', (np.str, 5))])

a = np.zeros(2, dtype=recordtype)

print(sys.getrefcount(a))
u, v = a[0], a[1]
u == v
del u, v
gc.collect()
print(sys.getrefcount(a))

The leak disappears if you remove the comparison (u == v) or if you remove the character string member in the dtype (the d member here). I've tried to diagnose this but I'm ending up completely lost in the comparison logic (which creates multiple temporary arrays).
@pitrou
Copy link
Member Author
pitrou commented Aug 26, 2015

Note this is a regression in 1.10. 1.9.2 is fine.

@ahaldane
Copy link
Member

I thought this might be due to structured array changes, but seems it has to do with string comparison.

Git bisect says it happens in 4b1f508 if I have done it correctly, but the fix isn't obvious to me. @njsmith, maybe you see something?

@pitrou
Copy link
Member Author
pitrou commented Aug 26, 2015

That must definitely be it. There is a missing Py_DECREF(array_other) after the call to _strings_richcompare.

It is astonishing the number of temporary arrays that get created here:

  • one temporary 0-d array is created for each scalar
  • one temporary array is created for each field in the structured dtype
  • and then one temporary array is created when comparing the string fields...

@njsmith
Copy link
Member
njsmith commented Aug 26, 2015

Yeah, that's definitely it. Good catch, thank you! I'll get a patch
together later today if no one beats be to it.
.
And yeah, that code is extremely inefficient. AFAICT the only way to fix it
though is to fix the issue with ufunc loops not having access to dtype
objects as discussed in the other thread, so that we can start using the
dtype machinery for string and void types.
On Aug 26, 2015 10:05 AM, "Antoine Pitrou" notifications@github.com wrote:

That must definitely be it. There is a missing Py_DECREF(array_other)
after the call to _strings_richcompare.

It is astonishing the number of temporary arrays that get created here:

  • one temporary 0-d array is created for each scalar
  • one temporary array is created for each field in the structured dtype
  • and then one temporary array is created when comparing the string
    fields...

—
Reply to this email directly or view it on GitHub
#6250 (comment).

@njsmith njsmith added this to the 1.10 blockers milestone Aug 27, 2015
njsmith added a commit to njsmith/numpy that referenced this issue Aug 27, 2015
@njsmith
BUG: Fix refcounting for string comparison in array_richcompare
02206a3 
Regression introduced in 4b1f508

Closes numpygh-6250.
@njsmith njsmith mentioned this issue Aug 27, 2015
Merged
@njsmith
Copy link
Member
njsmith commented Aug 27, 2015

[start using the ufunc machinery, I meant, in that last comment... i.e. instead of having this weird richcompare stuff we should just make np.equal work for these arrays, and the blocker on that is what I said.]

@pitrou
Copy link
Member Author
pitrou commented Aug 27, 2015

Well, the irony is that the code here compares scalars, so the indirection through arrays ideally shouldn't be necessary at all ;) That's a separate discussion anyway.

charris pushed a commit to charris/numpy that referenced this issue Aug 27, 2015
@njsmith @charris
BUG: Fix refcounting for string comparison in array_richcompare
2c74891 
Regression introduced in 4b1f508

Closes numpygh-6250.
jaimefrio pushed a commit to jaimefrio/numpy that referenced this issue Mar 22, 2016
@njsmith @jaimefrio
BUG: Fix refcounting for string comparison in array_richcompare
a9d1caa 
Regression introduced in 4b1f508

Closes numpygh-6250.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@njsmith @pitrou @ahaldane
0