Coder Social home page Coder Social logo

BufferError when using memoryview about yappi HOT 10 CLOSED

sumerc avatar sumerc commented on September 24, 2024
BufferError when using memoryview

from yappi.

Comments (10)

coldeasy avatar coldeasy commented on September 24, 2024 1

That's great, thanks for being so responsive. I'm happy to test your changes - currently I have this hack in place to unblock my progress

diff --git a/yappi/_yappi.c b/yappi/_yappi.c
index 7d21006..d8ea7f2 100644
--- a/yappi/_yappi.c
+++ b/yappi/_yappi.c
@@ -571,6 +571,11 @@ _ccode2pit(void *cco, uintptr_t current_tag)
         pit->builtin = 1;
         pit->modname = _pycfunction_module_name(cfn);
         pit->lineno = 0;
+       if (cfn->m_self && PyMemoryView_Check(cfn->m_self)) {
+           pit->fn_descriptor = PyStr_FromString("memoryview");
+           pit->name = PyStr_FromString("memoryview");
+           return pit;
+       }
         pit->fn_descriptor = (PyObject *)cfn;
         Py_INCREF(cfn);

from yappi.

coldeasy avatar coldeasy commented on September 24, 2024 1

Confirmed #61 fixes the issue, thanks @sumerc!

from yappi.

sumerc avatar sumerc commented on September 24, 2024

Wow. This is pretty interesting :) I will be looking into this.

from yappi.

sumerc avatar sumerc commented on September 24, 2024

The problem happens when we increment the reference count of the buildtin method of memoryview object in here:

Py_INCREF(cfn);

I have no idea why this leads to the error described. Maybe anyone has any idea?

Note to future: Once we understand the root cause, I will fix it maybe just by remove support for function selector for builtin functions? Not sure.

from yappi.

coldeasy avatar coldeasy commented on September 24, 2024

I believe it's because we cannot refer to the memoryview while the underlying structure (bytearray) changes shape. When we increment the reference count of a view we can violate this constraint. A non-yappi demonstration of this:

# import yappi
# yappi.start()


def test_mem():
    buf = bytearray()
    buf += b't' * 200
    # Note tobytes() has been removed
    view = memoryview(buf)[10:]
    del buf[:10]
    return view


if __name__ == "__main__":
    test_mem()

Here the lifetime of the view exceeds the lifetime of the original buf structure so we get the same BufferError.

from yappi.

sumerc avatar sumerc commented on September 24, 2024

Hmmm. I think you are right. Error message seemed a bit misleading, at least for me :) In fact we did not increment the view but one of the methods(tobytes() function) of the view but I assume that is same. We should not be touching the reference count of these kind of structures.

We increment the reference of every function being profiled in Yappi to support following convention:

def foo():
     pass

yappi.start()
foo()
stats = yappi.get_func_stats(
    filter_callback=lambda x: yappi.func_matches(x, [foo])
).print_all()

or in your case:

stats = yappi.get_func_stats(
    filter_callback=lambda x: yappi.func_matches(x, [memoryview.tobytes])
).print_all()

I am not sure about the fix, either.

Seeing these kind of issues, I am not sure if we include builtin functions for this kind of querying at all. I assume same kind of errors might happen as well for structures that hold references to other objects, somehow. Or maybe we could simply do not increment any ref. if the obj is memoryview.

Any idea on where this might break other than this situation?

from yappi.

coldeasy avatar coldeasy commented on September 24, 2024

The only thing that came to mind was items method on dictionaries but it looks like those views are dynamic and adapt when the dict is modified. I don't have any idea where else this issue might pop up, unfortunately.

I wonder is there some other method to implement the function equality checking. In your example we are checking equality against the class method - could we avoid supporting instance methods entirely and instead take a reference to the unbound method? Or is there a use-case where we need to keep instance method matching?

from yappi.

sumerc avatar sumerc commented on September 24, 2024

I wonder is there some other method to implement the function equality checking. In your example we are checking equality against the class method - could we avoid supporting instance methods entirely and instead take a reference to the unbound method?

I did not look at that option. Worth looking into!

from yappi.

sumerc avatar sumerc commented on September 24, 2024

Using method descriptors instead of PyCFunctionObject will prevent the possibility of selecting instance methods, but currently this does not seem like a big limitation.

And I think it is doable. I will fix for this on following days hopefully.

from yappi.

sumerc avatar sumerc commented on September 24, 2024

Hi again @coldeasy , I have managed to implement a fix for this issue. Could you please also verify on your end this works:
#61

Thanks!

from yappi.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.