◐ Shell
reader mode source ↗
Skip to content

bpo-36842: Implement PEP 578#12613

Merged
zooba merged 33 commits into
python:masterfrom
zooba:pep-578
May 23, 2019
Merged

bpo-36842: Implement PEP 578#12613
zooba merged 33 commits into
python:masterfrom
zooba:pep-578

Conversation

@zooba

@zooba zooba commented Mar 28, 2019

Copy link
Copy Markdown
Member

@zooba zooba marked this pull request as ready for review April 10, 2019 18:48
@zooba zooba requested a review from a team April 10, 2019 18:48
@zooba

zooba commented Apr 10, 2019

Copy link
Copy Markdown
Member Author

The PEP is not approved yet, of course, so the PR isn't finished. But I want it to at least run through CI now to catch anything that's going on.

@zooba zooba closed this Apr 10, 2019
@zooba zooba reopened this Apr 10, 2019
@brettcannon brettcannon removed the request for review from a team April 17, 2019 22:46
@brettcannon

Copy link
Copy Markdown
Member

Removing the import team from reviewing as this is still a WIP. We can be added back when it's ready to go.

@zooba zooba changed the title Implement PEP 578 May 7, 2019
@zooba zooba requested a review from a team as a code owner May 8, 2019 15:29
66 hidden items Load more…
1 hidden conversation Load more…
@vstinner

Copy link
Copy Markdown
Member

Patch to use METH_FASTCALL:

diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index f987bb2016..fd1408c748 100644
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -22,6 +22,7 @@ Data members:
 #include "pycore_pymem.h"
 #include "pycore_pathconfig.h"
 #include "pycore_pystate.h"
+#include "pycore_tupleobject.h"
 #include "pythread.h"
 #include "pydtrace.h"
 
@@ -360,20 +361,14 @@ PyDoc_STRVAR(audit_doc,
 Passes the event to any audit hooks that are attached.");
 
 static PyObject *
-sys_audit(PyObject *self, PyObject *args)
+sys_audit(PyObject *self, PyObject *const *args, Py_ssize_t argc)
 {
-    if (!PyTuple_Check(args)) {
-        PyErr_Format(PyExc_TypeError, "expected tuple, not %.200s", Py_TYPE(args)->tp_name);
-        return NULL;
-    }
-
-    Py_ssize_t argc = PyTuple_GET_SIZE(args);
     if (argc == 0) {
         PyErr_SetString(PyExc_TypeError, "audit() missing 1 required positional argument: 'event'");
         return NULL;
     }
 
-    PyObject *auditEvent = PyTuple_GET_ITEM(args, 0);
+    PyObject *auditEvent = args[0];
     if (!auditEvent) {
         PyErr_SetString(PyExc_TypeError, "expected str for argument 'event'");
         return NULL;
@@ -388,16 +383,10 @@ sys_audit(PyObject *self, PyObject *args)
         return NULL;
     }
 
-    PyObject *auditArgs = PyTuple_New(argc - 1);
+    PyObject *auditArgs = _PyTuple_FromArray(args + 1, argc - 1);
     if (!auditArgs) {
         return NULL;
     }
-    for (int i = 0; i < argc - 1; ++i) {
-        PyObject *o = PyTuple_GET_ITEM(args, i + 1);
-        Py_INCREF(o);
-        PyTuple_SET_ITEM(auditArgs, i, o);
-    }
-
     int res = PySys_Audit(event, "O", auditArgs);
     Py_DECREF(auditArgs);
 
@@ -1924,7 +1913,7 @@ sys_getandroidapilevel_impl(PyObject *module)
 static PyMethodDef sys_methods[] = {
     /* Might as well keep this in alphabetic order */
     SYS_ADDAUDITHOOK_METHODDEF
-    {"audit",           sys_audit, METH_VARARGS, audit_doc },
+    {"audit",           (PyCFunction)(void(*)(void))sys_audit, METH_FASTCALL, audit_doc },
     {"breakpointhook",  (PyCFunction)(void(*)(void))sys_breakpointhook,
      METH_FASTCALL | METH_KEYWORDS, breakpointhook_doc},
     SYS_CALLSTATS_METHODDEF

By the way, I might be useful to extract the code from PySys_Audit() to decide if audit is used or not: create a subfunction, and call it from sys_audit() to do nothing if audit is not used (common case).

Calling PyUnicode_AsUTF8() and _PyTuple_FromArray() is not free :-) (I know that they are fast, but many function calls take less than 100 ns overall.)

@zooba

zooba commented May 15, 2019

Copy link
Copy Markdown
Member Author

@tiran I believe all feedback has been addressed, and all tests pass (custom buildbot run going now). Anything else?

@zooba

zooba commented May 17, 2019

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

@zooba

zooba commented May 21, 2019

Copy link
Copy Markdown
Member Author

@tiran I know you're busy with your PEP, but this is waiting on you.

I'll give it two more merge conflicts from other commits before I consider your feedback addressed and merge :)

@zooba

zooba commented May 23, 2019

Copy link
Copy Markdown
Member Author

That's the second conflict, so I'm dismissing @tiran's review and we can deal with any further issues post-commit.

@zooba zooba dismissed tiran’s stale review May 23, 2019 15:22

E_TIMEOUT

@zooba zooba merged commit b82e17e into python:master May 23, 2019
@zooba zooba deleted the pep-578 branch May 23, 2019 15:45
zooba added a commit to zooba/cpython that referenced this pull request Aug 21, 2019
Adds sys.audit, sys.addaudithook, io.open_code, and associated C APIs.
@zooba zooba mentioned this pull request Jun 17, 2023
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.

7 participants