On Fri, Jun 03, 2022 at 05:26:34PM -0500, Eric Blake wrote:
Instead of storing a PyCapsule in _o and repeatedly doing lookups to dereference a stored malloc'd pointer, it is easier to just directly store a Python buffer-like object as _o. Then, instead of using Py_{INC,DEC}REF across the paired aio_p{read,write} and corresponding completion function, we now rely on PyObject_GetBuffer and ByBuffer_Release. The use of memoryview protects us from the python user changing the buffer out from under our feet:
$ ./run nbdsh nbd> h.connect_command(['nbdkit','-s','memory','10']) nbd> b = bytearray(10) nbd> b1 = nbd.Buffer.from_bytearray(b) nbd> h.aio_pread(b1, 0) 1 nbd> b.pop() Traceback (most recent call last): File "/usr/lib64/python3.10/code.py", line 90, in runcode exec(code, self.locals) File "<console>", line 1, in <module> BufferError: Existing exports of data: object cannot be re-sized nbd> h.poll(-1) 1 nbd> h.aio_command_completed(1) True nbd> b1 = None nbd> b.pop() 0
This ALSO means that we're doing less copying! now that nbd.Buffer.from_bytearray() is reusing an existing python object instead of copying to a C malloc'd buffer, we have noticeable effects on performance. For example, on my machine:
$ export script=' m=1024*1024 size=h.get_size() zero=bytearray(m) for i in range(size // m): c = h.aio_pwrite(nbd.Buffer.from_bytearray(zero), m*i) while not h.aio_command_completed(c): h.poll(-1) ' $ time nbdkit -U - null 10G --run 'nbdsh -u $uri -c "$script"'
takes 2.9s pre-patch, and 2.1s post-patch.
I noticed that nbd.Buffer(-1) changes from: RuntimeError: length < 0 to SystemError: Negative size passed to PyByteArray_FromStringAndSize It would not be hard to revert that part, if needed.
The new message isn't difficult to understand.
Pure Python doesn't have any quick isinstance() test for whether an object is buffer-like (only C has that, in PyObject_CheckBuffer) [1]. That makes it interesting to preserve our pre-existing semantics (from the recent commit d477f7c7) where nbd.Buffer(int) gives uninitialized memory, but nbd.Buffer.from_bytearray(int) relies on the bytearray(int) constructor to give us initialized memory. For the constructor, I had to use C, but for .from_bytearray (which is now slightly misnamed, oh well), I chose to stick to a python try/except instead of deferring to a C helper function.
[1] https://stackoverflow.com/questions/30017991/check-if-an-object-supports-the...
The generated code changes as follows:
| --- python/methods.h.bak 2022-06-03 17:10:24.533842689 -0500 | +++ python/methods.h 2022-06-03 17:10:33.833858093 -0500 | @@ -28,16 +28,11 @@ | | #include <assert.h> | | -struct py_aio_buffer { | - Py_ssize_t len; | - void *data; | -}; | - | extern char **nbd_internal_py_get_string_list (PyObject *); | extern void nbd_internal_py_free_string_list (char **); | extern int nbd_internal_py_get_sockaddr (PyObject *, | struct sockaddr_storage *, socklen_t *); | -extern struct py_aio_buffer *nbd_internal_py_get_aio_buffer (PyObject *); | +extern PyObject *nbd_internal_py_get_aio_buffer (PyObject *); | extern PyObject *nbd_internal_py_get_nbd_buffer_type (void); | | static inline struct nbd_handle * | @@ -66,9 +61,6 @@ | extern PyObject *nbd_internal_py_close (PyObject *self, PyObject *args); | extern PyObject *nbd_internal_py_display_version (PyObject *self, PyObject *args); | extern PyObject *nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args); | -extern PyObject *nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args); | -extern PyObject *nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args); | -extern PyObject *nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args); | extern PyObject *nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args); | extern PyObject *nbd_internal_py_set_debug (PyObject *self, PyObject *args); | extern PyObject *nbd_internal_py_get_debug (PyObject *self, PyObject *args); | --- python/methods.c.bak 2022-06-03 17:10:25.741844689 -0500 | +++ python/methods.c 2022-06-03 17:13:09.689116273 -0500 | @@ -37,7 +37,7 @@ | */ | struct user_data { | PyObject *fn; /* Optional pointer to Python function. */ | - PyObject *buf; /* Optional pointer to persistent buffer. */ | + Py_buffer view; /* persistent buffer, if view->obj set. */ | }; | | static struct user_data * | @@ -58,7 +58,7 @@ free_user_data (void *user_data) | | if (data) { | Py_XDECREF (data->fn); | - Py_XDECREF (data->buf); | + PyBuffer_Release(&data->view); | free (data); | } | } | @@ -3163,7 +3163,7 @@ nbd_internal_py_aio_pread (PyObject *sel | int64_t ret; | PyObject *py_ret = NULL; | PyObject *buf; /* instance of nbd.Buffer */ | - struct py_aio_buffer *buf_buf; /* Contents of nbd.Buffer */ | + PyObject *buf_buf; /* Contents of nbd.Buffer */ | uint64_t offset_u64; | unsigned long long offset; /* really uint64_t */ | struct user_data *completion_user_data = NULL; | @@ -3196,12 +3196,11 @@ nbd_internal_py_aio_pread (PyObject *sel | flags_u32 = flags; | buf_buf = nbd_internal_py_get_aio_buffer (buf); | if (!buf_buf) goto out; | - /* Increment refcount since buffer may be saved by libnbd. */ | - Py_INCREF (buf); | - completion_user_data->buf = buf; | + if (PyObject_GetBuffer(buf_buf, &completion_user_data->view, | + PyBUF_CONTIG) < 0) goto out; | offset_u64 = offset; | | - ret = nbd_aio_pread (h, buf_buf->data, buf_buf->len, offset_u64, completion, flags_u32); | + ret = nbd_aio_pread (h, completion_user_data->view.buf, completion_user_data->view.len, offset_u64, completion, flags_u32); | completion_user_data = NULL; | if (ret == -1) { | raise_exception (); | @@ -3210,6 +3209,7 @@ nbd_internal_py_aio_pread (PyObject *sel | py_ret = PyLong_FromLongLong (ret); | | out: | + Py_XDECREF (buf_buf); | free_user_data (completion_user_data); | return py_ret; | } | @@ -3222,7 +3222,7 @@ nbd_internal_py_aio_pread_structured (Py | int64_t ret; | PyObject *py_ret = NULL; | PyObject *buf; /* instance of nbd.Buffer */ | - struct py_aio_buffer *buf_buf; /* Contents of nbd.Buffer */ | + PyObject *buf_buf; /* Contents of nbd.Buffer */ | uint64_t offset_u64; | unsigned long long offset; /* really uint64_t */ | struct user_data *chunk_user_data = NULL; | @@ -3259,9 +3259,8 @@ nbd_internal_py_aio_pread_structured (Py | flags_u32 = flags; | buf_buf = nbd_internal_py_get_aio_buffer (buf); | if (!buf_buf) goto out; | - /* Increment refcount since buffer may be saved by libnbd. */ | - Py_INCREF (buf); | - completion_user_data->buf = buf; | + if (PyObject_GetBuffer(buf_buf, &completion_user_data->view, | + PyBUF_CONTIG) < 0) goto out; | offset_u64 = offset; | chunk.user_data = chunk_user_data = alloc_user_data (); | if (chunk_user_data == NULL) goto out; | @@ -3274,7 +3273,7 @@ nbd_internal_py_aio_pread_structured (Py | Py_INCREF (py_chunk_fn); | chunk_user_data->fn = py_chunk_fn; | | - ret = nbd_aio_pread_structured (h, buf_buf->data, buf_buf->len, offset_u64, chunk, completion, flags_u32); | + ret = nbd_aio_pread_structured (h, completion_user_data->view.buf, completion_user_data->view.len, offset_u64, chunk, completion, flags_u32);
Could use pr_wrap ',' to wrap this nicely, although the old code wasn't wrapped nicely either.
| chunk_user_data = NULL; | completion_user_data = NULL; | if (ret == -1) { | @@ -3284,6 +3283,7 @@ nbd_internal_py_aio_pread_structured (Py | py_ret = PyLong_FromLongLong (ret); | | out: | + Py_XDECREF (buf_buf); | free_user_data (chunk_user_data); | free_user_data (completion_user_data); | return py_ret; | @@ -3297,7 +3297,7 @@ nbd_internal_py_aio_pwrite (PyObject *se | int64_t ret; | PyObject *py_ret = NULL; | PyObject *buf; /* instance of nbd.Buffer */ | - struct py_aio_buffer *buf_buf; /* Contents of nbd.Buffer */ | + PyObject *buf_buf; /* Contents of nbd.Buffer */ | uint64_t offset_u64; | unsigned long long offset; /* really uint64_t */ | struct user_data *completion_user_data = NULL; | @@ -3330,12 +3330,11 @@ nbd_internal_py_aio_pwrite (PyObject *se | flags_u32 = flags; | buf_buf = nbd_internal_py_get_aio_buffer (buf); | if (!buf_buf) goto out; | - /* Increment refcount since buffer may be saved by libnbd. */ | - Py_INCREF (buf); | - completion_user_data->buf = buf; | + if (PyObject_GetBuffer(buf_buf, &completion_user_data->view, | + PyBUF_CONTIG_RO) < 0) goto out; | offset_u64 = offset; | | - ret = nbd_aio_pwrite (h, buf_buf->data, buf_buf->len, offset_u64, completion, flags_u32); | + ret = nbd_aio_pwrite (h, completion_user_data->view.buf, completion_user_data->view.len, offset_u64, completion, flags_u32); | completion_user_data = NULL; | if (ret == -1) { | raise_exception (); | @@ -3344,6 +3343,7 @@ nbd_internal_py_aio_pwrite (PyObject *se | py_ret = PyLong_FromLongLong (ret); | | out: | + Py_XDECREF (buf_buf); | free_user_data (completion_user_data); | return py_ret; | } | --- python/nbd.py.bak 2022-06-03 17:10:23.350840729 -0500 | +++ python/nbd.py 2022-06-03 17:10:33.852858124 -0500 | @@ -128,19 +128,21 @@ class Buffer(object): | | @classmethod | def from_bytearray(cls, ba): | - '''create an AIO buffer from a bytearray''' | - o = libnbdmod.aio_buffer_from_bytearray(ba) | + '''create an AIO buffer from a bytearray or other buffer''' | self = cls(0) | - self._o = o | + try: | + self._o = memoryview(ba).cast('B') | + except TypeError: | + self._o = bytearray(ba) | return self | | def to_bytearray(self): | '''copy an AIO buffer into a bytearray''' | - return libnbdmod.aio_buffer_to_bytearray(self) | + return bytearray(self._o) | | def size(self): | '''return the size of an AIO buffer''' | - return libnbdmod.aio_buffer_size(self) | + return len(self._o) | | def is_zero(self, offset=0, size=-1): | ''' | @@ -154,7 +156,7 @@ class Buffer(object): | always returns true. If size > 0, we check the interval | [offset..offset+size-1]. | ''' | - return libnbdmod.aio_buffer_is_zero(self, offset, size) | + return libnbdmod.aio_buffer_is_zero(self._o, offset, size) | | | class NBD(object): --- generator/Python.ml | 52 ++++++------ python/handle.c | 188 +++++++------------------------------------- 2 files changed, 52 insertions(+), 188 deletions(-)
Nice simplification!
diff --git a/generator/Python.ml b/generator/Python.ml index b862b44..270858f 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -34,16 +34,11 @@ let pr "#include <assert.h>\n"; pr "\n"; pr "\ -struct py_aio_buffer { - Py_ssize_t len; - void *data; -}; - extern char **nbd_internal_py_get_string_list (PyObject *); extern void nbd_internal_py_free_string_list (char **); extern int nbd_internal_py_get_sockaddr (PyObject *, struct sockaddr_storage *, socklen_t *); -extern struct py_aio_buffer *nbd_internal_py_get_aio_buffer (PyObject *); +extern PyObject *nbd_internal_py_get_aio_buffer (PyObject *); extern PyObject *nbd_internal_py_get_nbd_buffer_type (void);
static inline struct nbd_handle * @@ -77,9 +72,6 @@ let ) ([ "create"; "close"; "display_version"; "alloc_aio_buffer"; - "aio_buffer_from_bytearray"; - "aio_buffer_to_bytearray"; - "aio_buffer_size"; "aio_buffer_is_zero" ] @ List.map fst handle_calls);
pr "\n"; @@ -109,9 +101,6 @@ let ) ([ "create"; "close"; "display_version"; "alloc_aio_buffer"; - "aio_buffer_from_bytearray"; - "aio_buffer_to_bytearray"; - "aio_buffer_size"; "aio_buffer_is_zero" ] @ List.map fst handle_calls); pr " { NULL, NULL, 0, NULL }\n"; pr "};\n"; @@ -301,7 +290,7 @@ let | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> pr " PyObject *%s; /* instance of nbd.Buffer */\n" n; - pr " struct py_aio_buffer *%s_buf; /* Contents of nbd.Buffer */\n" n + pr " PyObject *%s_buf; /* Contents of nbd.Buffer */\n" n | Closure { cbname } -> pr " struct user_data *%s_user_data = NULL;\n" cbname; pr " PyObject *py_%s_fn;\n" cbname; @@ -436,12 +425,16 @@ let | BytesOut (n, count) -> pr " %s = PyByteArray_FromStringAndSize (NULL, %s);\n" n count; pr " if (%s == NULL) goto out;\n" n - | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> + | BytesPersistIn (n, _) -> pr " %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n; pr " if (!%s_buf) goto out;\n" n; - pr " /* Increment refcount since buffer may be saved by libnbd. */\n"; - pr " Py_INCREF (%s);\n" n; - pr " completion_user_data->buf = %s;\n" n + pr " if (PyObject_GetBuffer(%s_buf, &completion_user_data->view,\n" n; + pr " PyBUF_CONTIG_RO) < 0) goto out;\n" + | BytesPersistOut (n, _) -> + pr " %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n; + pr " if (!%s_buf) goto out;\n" n; + pr " if (PyObject_GetBuffer(%s_buf, &completion_user_data->view,\n" n; + pr " PyBUF_CONTIG) < 0) goto out;\n" | Closure { cbname } -> pr " %s.user_data = %s_user_data = alloc_user_data ();\n" cbname cbname; pr " if (%s_user_data == NULL) goto out;\n" cbname; @@ -482,8 +475,8 @@ let | Bool n -> pr ", %s" n | BytesIn (n, _) -> pr ", %s.buf, %s.len" n n | BytesOut (n, count) -> pr ", PyByteArray_AS_STRING (%s), %s" n count - | BytesPersistIn (n, _) - | BytesPersistOut (n, _) -> pr ", %s_buf->data, %s_buf->len" n n + | BytesPersistIn _ | BytesPersistOut _ -> + pr ", completion_user_data->view.buf, completion_user_data->view.len" | Closure { cbname } -> pr ", %s" cbname | Enum (n, _) -> pr ", %s" n | Flags (n, _) -> pr ", %s_u32" n @@ -576,7 +569,8 @@ let pr " if (%s.obj)\n" n; pr " PyBuffer_Release (&%s);\n" n | BytesOut (n, _) -> pr " Py_XDECREF (%s);\n" n - | BytesPersistIn _ | BytesPersistOut _ -> () + | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> + pr " Py_XDECREF (%s_buf);\n" n | Closure { cbname } -> pr " free_user_data (%s_user_data);\n" cbname | Enum _ -> () @@ -625,7 +619,7 @@ let pr " */\n"; pr "struct user_data {\n"; pr " PyObject *fn; /* Optional pointer to Python function. */\n"; - pr " PyObject *buf; /* Optional pointer to persistent buffer. */\n"; + pr " Py_buffer view; /* persistent buffer, if view->obj set. */\n"; pr "};\n"; pr "\n"; pr "static struct user_data *\n"; @@ -646,7 +640,7 @@ let pr "\n"; pr " if (data) {\n"; pr " Py_XDECREF (data->fn);\n"; - pr " Py_XDECREF (data->buf);\n"; + pr " PyBuffer_Release(&data->view);\n"; pr " free (data);\n"; pr " }\n"; pr "}\n"; @@ -768,19 +762,21 @@ let
@classmethod def from_bytearray(cls, ba): - '''create an AIO buffer from a bytearray''' - o = libnbdmod.aio_buffer_from_bytearray(ba) + '''create an AIO buffer from a bytearray or other buffer''' self = cls(0) - self._o = o + try: + self._o = memoryview(ba).cast('B') + except TypeError: + self._o = bytearray(ba) return self
def to_bytearray(self): '''copy an AIO buffer into a bytearray''' - return libnbdmod.aio_buffer_to_bytearray(self) + return bytearray(self._o)
def size(self): '''return the size of an AIO buffer''' - return libnbdmod.aio_buffer_size(self) + return len(self._o)
def is_zero(self, offset=0, size=-1): ''' @@ -794,7 +790,7 @@ let always returns true. If size > 0, we check the interval [offset..offset+size-1]. ''' - return libnbdmod.aio_buffer_is_zero(self, offset, size) + return libnbdmod.aio_buffer_is_zero(self._o, offset, size)
class NBD(object): diff --git a/python/handle.c b/python/handle.c index f84c6e0..7f67159 100644 --- a/python/handle.c +++ b/python/handle.c @@ -98,205 +98,73 @@ nbd_internal_py_display_version (PyObject *self, PyObject *args)
static const char aio_buffer_name[] = "nbd.Buffer";
-struct py_aio_buffer * +PyObject * nbd_internal_py_get_aio_buffer (PyObject *buffer) { - if (PyObject_IsInstance (buffer, nbd_internal_py_get_nbd_buffer_type ())) { - PyObject *capsule = PyObject_GetAttrString(buffer, "_o"); - return PyCapsule_GetPointer (capsule, aio_buffer_name); - } + if (PyObject_IsInstance (buffer, nbd_internal_py_get_nbd_buffer_type ())) + return PyObject_GetAttrString(buffer, "_o");
PyErr_SetString (PyExc_TypeError, "aio_buffer: expecting nbd.Buffer instance"); return NULL; }
-static void -free_aio_buffer (PyObject *capsule) -{ - struct py_aio_buffer *buf = PyCapsule_GetPointer (capsule, aio_buffer_name); - - if (buf) - free (buf->data); - free (buf); -} - /* Allocate a persistent buffer used for nbd_aio_pread. */ PyObject * nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args) { - struct py_aio_buffer *buf; - PyObject *ret; - - buf = malloc (sizeof *buf); - if (buf == NULL) { - PyErr_NoMemory (); - return NULL; - } - - if (!PyArg_ParseTuple (args, (char *) "n:nbd_internal_py_alloc_aio_buffer", - &buf->len)) { - free (buf); - return NULL; - } - - if (buf->len < 0) { - PyErr_SetString (PyExc_RuntimeError, "length < 0"); - free (buf); - return NULL; - } - buf->data = malloc (buf->len); - if (buf->data == NULL) { - PyErr_NoMemory (); - free (buf); - return NULL; - } - - ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer); - if (ret == NULL) { - free (buf->data); - free (buf); - return NULL; - } - - return ret; -} - -PyObject * -nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args) -{ - PyObject *obj; - PyObject *arr = NULL; Py_ssize_t len; - void *data; - struct py_aio_buffer *buf; - PyObject *ret;
- if (!PyArg_ParseTuple (args, - (char *) "O:nbd_internal_py_aio_buffer_from_bytearray", - &obj)) + if (!PyArg_ParseTuple (args, (char *) "n:nbd_internal_py_alloc_aio_buffer", + &len)) return NULL;
- if (! PyByteArray_Check (obj)) { - arr = PyByteArray_FromObject (obj); - if (arr == NULL) - return NULL; - obj = arr; - } - data = PyByteArray_AsString (obj); - if (!data) { - PyErr_SetString (PyExc_RuntimeError, - "parameter is not a bytearray or buffer"); - Py_XDECREF (arr); - return NULL; - } - len = PyByteArray_Size (obj); - - buf = malloc (sizeof *buf); - if (buf == NULL) { - PyErr_NoMemory (); - Py_XDECREF (arr); - return NULL; - } - - buf->len = len; - buf->data = malloc (len); - if (buf->data == NULL) { - PyErr_NoMemory (); - free (buf); - Py_XDECREF (arr); - return NULL; - } - memcpy (buf->data, data, len); - Py_XDECREF (arr); - - ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer); - if (ret == NULL) { - free (buf->data); - free (buf); - return NULL; - } - - return ret; -} - -PyObject * -nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args) -{ - PyObject *obj; - struct py_aio_buffer *buf; - - if (!PyArg_ParseTuple (args, - (char *) "O:nbd_internal_py_aio_buffer_to_bytearray", - &obj)) - return NULL; - - buf = nbd_internal_py_get_aio_buffer (obj); - if (buf == NULL) - return NULL; - - return PyByteArray_FromStringAndSize (buf->data, buf->len); -} - -PyObject * -nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args) -{ - PyObject *obj; - struct py_aio_buffer *buf; - - if (!PyArg_ParseTuple (args, - (char *) "O:nbd_internal_py_aio_buffer_size", - &obj)) - return NULL; - - buf = nbd_internal_py_get_aio_buffer (obj); - if (buf == NULL) - return NULL; - - return PyLong_FromSsize_t (buf->len); + /* Constructing bytearray(len) in python zeroes the memory; doing it this + * way gives uninitialized memory. This correctly flags negative len. + */ + return PyByteArray_FromStringAndSize (NULL, len); }
PyObject * nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args) { - PyObject *obj; - struct py_aio_buffer *buf; + Py_buffer buf; Py_ssize_t offset, size; + PyObject *ret = NULL;
if (!PyArg_ParseTuple (args, - (char *) "Onn:nbd_internal_py_aio_buffer_is_zero", - &obj, &offset, &size)) + (char *) "y*nn:nbd_internal_py_aio_buffer_is_zero", + &buf, &offset, &size)) return NULL;
- if (size == 0) - Py_RETURN_TRUE; - - buf = nbd_internal_py_get_aio_buffer (obj); - if (buf == NULL) - return NULL; + if (size == 0) { + ret = Py_True; + Py_INCREF (ret); + goto out; + }
/* Check the bounds of the offset. */ - if (offset < 0 || offset > buf->len) { + if (offset < 0 || offset > buf.len) { PyErr_SetString (PyExc_IndexError, "offset out of range"); - return NULL; + goto out; }
/* Compute or check the length. */ if (size == -1) - size = buf->len - offset; + size = buf.len - offset; else if (size < 0) { PyErr_SetString (PyExc_IndexError, "size cannot be negative, " "except -1 to mean to the end of the buffer"); - return NULL; + goto out; } - else if ((size_t) offset + size > buf->len) { + else if ((size_t) offset + size > buf.len) { PyErr_SetString (PyExc_IndexError, "size out of range"); - return NULL; + goto out; }
- if (is_zero (buf->data + offset, size)) - Py_RETURN_TRUE; - else - Py_RETURN_FALSE; + ret = PyBool_FromLong (is_zero (buf.buf + offset, size)); + out: + PyBuffer_Release(&buf); + return ret; } -- 2.36.1
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
Reviewed-by: Richard W.M. Jones <rjones@redhat.com> -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit