ronantakizawa/codereview-bench huggingface dataset:
Repository: nsqio/nsq
File: dist.sh (Shell)

Code before:
export GO111MODULE=on
for target in "linux/amd64" "linux/arm64" "darwin/amd64" "darwin/arm64" "freebsd/amd64" "windows/amd64"; do
    os=${target%/*}
    arch=${target##*/}
    echo "... building v$version for $os/$arch"
    BUILD=$(mktemp -d ${TMPDIR:-/tmp}/nsq-XXXXX)
    TARGET="nsq-$version.$os-$arch.$goversion"
    GOOS=$os GOARCH=$arch CGO_ENABLED=0 \
        make DESTDIR=$BUILD PREFIX=/$TARGET BLDFLAGS="$GOFLAGS" install
    pushd $BUILD
    sudo chown -R 0:0 $TARGET
    tar czvf $TARGET.tar.gz $TARGET
    mv $TARGET.tar.gz $DIR/dist
    popd
    make clean
    sudo rm -r $BUILD
done

rnd=$(LC_ALL=C tr -dc 'a-zA-Z0-9' < /dev/urandom | head -c10)
docker buildx create --use --name nsq-$rnd
docker buildx build --tag nsqio/nsq:v$version . --platform linux/amd64,linux/arm64 --push
if [[ ! $version == *"-"* ]]; then
    echo "Tagging nsqio/nsq:v$version as the latest release."
    docker buildx build --tag nsqio/nsq:latest . --platform linux/amd64,linux/arm64 --push
fi
docker buildx rm nsq-$rnd


Review comment (suggestion):
i think without the `buildx rm` the builders will pile up.  not sure how much resources they use.  

it might be possible to do an intelligent builder cleanup prior to setting up new builders using `docker buildx ls` and interpreting the results.  for now on our systems if something happens and the builder gets orphan'd, i've just been removing it manually.

in our circle-ci setup on sso ( https://github.com/buzzfeed/sso/pull/319 ) since we use ephemeral build machines we really don't have to care about the orphan builders.

Code after:
export GO111MODULE=on
for target in "linux/amd64" "linux/arm64" "darwin/amd64" "darwin/arm64" "freebsd/amd64" "windows/amd64"; do
    os=${target%/*}
    arch=${target##*/}
    echo "... building v$version for $os/$arch"
    BUILD=$(mktemp -d ${TMPDIR:-/tmp}/nsq-XXXXX)
    TARGET="nsq-$version.$os-$arch.$goversion"
    GOOS=$os GOARCH=$arch CGO_ENABLED=0 \
        make DESTDIR=$BUILD PREFIX=/$TARGET BLDFLAGS="$GOFLAGS" install
    pushd $BUILD
    sudo chown -R 0:0 $TARGET
    tar czvf $TARGET.tar.gz $TARGET
    mv $TARGET.tar.gz $DIR/dist
    popd
    make clean
    sudo rm -r $BUILD
done

rnd=$(LC_ALL=C tr -dc 'a-zA-Z0-9' < /dev/urandom | head -c10)
docker buildx create --use --name nsq-$rnd
docker buildx build --tag nsqio/nsq:v$version --platform linux/amd64,linux/arm64 .
if [[ ! $version == *"-"* ]]; then
    echo "Tagging nsqio/nsq:v$version as the latest release"
    shas=$(docker manifest inspect nsqio/nsq:$version |\
        grep digest | awk '{print $2}' | sed 's/[",]//g' | sed 's/^/nsqio\/nsq@/')
    docker manifest create nsqio/nsq:latest $shas
fi
Repository: nsqio/nsq
File: nsqd/topic.go (Go)

Code before:

	for i, m := range msgs {
		err := t.put(m)
		if err != nil {
			atomic.AddUint64(&t.messageCount, uint64(i))
			atomic.AddUint64(&t.messageBytes, uint64(messageTotalBytes))
			return err
		}
		messageTotalBytes += len(m.Body)
	}

	atomic.AddUint64(&t.messageBytes, uint64(messageTotalBytes))
	atomic.AddUint64(&t.messageCount, uint64(len(msgs)))
	return nil
}

func (t *Topic) put(m *Message) error {
	// if mem-queue-size == 0, avoid memory chan, for more consistent ordering
	if cap(t.memoryMsgChan) > 0 {
		select {
		case t.memoryMsgChan <- m:
			return nil
		default:
			break // write to backend
		}
	} else if t.ephemeral || m.deferred != 0 {
		// try extra hard to deliver to channels through memory chan because
		// messages lose deferred time in backend, and ephemeral topic has no backend
		if len(t.channelMap) > 0 && !t.IsPaused() && t.IsStarted() {
			select {
			case t.memoryMsgChan <- m:
				return nil
			case <-time.After(time.Second):
				break // give up
			}
		}
	}
	err := writeMessageToBackend(m, t.backend)
	t.nsqd.SetHealth(err)
	if err != nil {
		t.nsqd.logf(LOG_ERROR,
			"TOPIC(%s) ERROR: failed to write message to backend - %s",
			t.name, err)
		return err
	}
	return nil
}

func (t *Topic) Depth() int64 {
	return int64(len(t.memoryMsgChan)) + t.backend.Depth()
}

Review comment (refactor):
I don't think we want to wait around for a second in the ephemeral case - the intention is that it's bounded and lossy, why add back pressure when the queue is full?

My proposal would be to simplify this whole if/else block to:

```go
if cap(t.memoryMsgChan) > 0 || m.deferred != 0 {
		select {
		case t.memoryMsgChan <- m:
			return nil
		default:
			break // write to backend
		}
}
```

Code after:
		err := t.put(m)
		if err != nil {
			atomic.AddUint64(&t.messageCount, uint64(i))
			atomic.AddUint64(&t.messageBytes, uint64(messageTotalBytes))
			return err
		}
		messageTotalBytes += len(m.Body)
	}

	atomic.AddUint64(&t.messageBytes, uint64(messageTotalBytes))
	atomic.AddUint64(&t.messageCount, uint64(len(msgs)))
	return nil
}

func (t *Topic) put(m *Message) error {
	// If mem-queue-size == 0, avoid memory chan, for more consistent ordering,
	// but try to use memory chan for deferred messages (they lose deferred timer
	// in backend queue) or if topic is ephemeral (there is no backend queue).
	if cap(t.memoryMsgChan) > 0 || t.ephemeral || m.deferred != 0 {
		select {
		case t.memoryMsgChan <- m:
			return nil
		default:
			break // write to backend
		}
	}
	err := writeMessageToBackend(m, t.backend)
	t.nsqd.SetHealth(err)
	if err != nil {
		t.nsqd.logf(LOG_ERROR,
			"TOPIC(%s) ERROR: failed to write message to backend - %s",
			t.name, err)
		return err
	}
	return nil
}

func (t *Topic) Depth() int64 {
	return int64(len(t.memoryMsgChan)) + t.backend.Depth()
}

// messagePump selects over the in-memory and backend queue and
// writes messages to every channel for this topic
func (t *Topic) messagePump() {
	var msg *Message
	var buf []byte
	var err error
	var chans []*Channel
	var memoryMsgChan chan *Message
	var backendChan <-chan []byte
Repository: numpy/numpy
File: numpy/core/src/multiarray/ctors.c (C)

Code before:
                goto fail;
            }
        }

        /* dst points to correct array subsection */
        if (PyArray_CopyInto(dst, (PyArrayObject *)s) < 0) {
            goto fail;
        }

        Py_DECREF(s);
        return 0;
    }

    if (dim > PyArray_NDIM(a)) {
        PyErr_Format(PyExc_ValueError,
                 "setArrayFromSequence: sequence/array dimensions mismatch.");
        goto fail;
    }

    slen = PySequence_Length(s);
    if (slen < 0) {
        goto fail;
    }
    /*
     * Either the dimensions match, or the sequence has length 1 and can
     * be broadcast to the destination.
     */
    if (slen != PyArray_DIMS(a)[dim] && slen != 1) {
        PyErr_Format(PyExc_ValueError,
                 "cannot copy sequence with size %zd to array axis "
                 "with dimension %" NPY_INTP_FMT, slen, PyArray_DIMS(a)[dim]);
        goto fail;
    }

    /* Broadcast the one element from the sequence to all the outputs */
    if (slen == 1) {
        PyObject *o;
        npy_intp alen = PyArray_DIM(a, dim);

        o = PySequence_GetItem(s, 0);
        if (o == NULL) {
            goto fail;
        }

        for (i = 0; i < alen; i++) {
            if ((PyArray_NDIM(a) - dim) > 1) {
                PyArrayObject * tmp =
                    (PyArrayObject *)array_item_asarray(dst, i);
                if (tmp == NULL) {
                    goto fail;
                }

Review comment (suggestion):
nit: Tempting to change this to match the other call exactly, either as `_array_from_array_like(obj, NULL, NPY_FALSE, NULL);` here or this way there.

Code after:
            /* This will DECREF(s) if replaced */
            s = PyArray_EnsureArray(s);
            if (s == NULL) {
                goto fail;
            }
        }

        /* dst points to correct array subsection */
        if (PyArray_CopyInto(dst, (PyArrayObject *)s) < 0) {
            goto fail;
        }

        Py_DECREF(s);
        return 0;
    }

    if (dim > PyArray_NDIM(a)) {
        PyErr_Format(PyExc_ValueError,
                 "setArrayFromSequence: sequence/array dimensions mismatch.");
        goto fail;
    }

    /* Try __array__ before using s as a sequence */
    PyObject *tmp = _array_from_array_like(s, NULL, 0, NULL);
    if (tmp == NULL) {
        goto fail;
    }
    else if (tmp == Py_NotImplemented) {
        Py_DECREF(tmp);
    }
    else {
        int r = PyArray_CopyInto(dst, (PyArrayObject *)tmp);
        Py_DECREF(tmp);
        if (r < 0) {
            goto fail;
        }
        Py_DECREF(s);
        return 0;
    }

    slen = PySequence_Length(s);
    if (slen < 0) {
        goto fail;
    }

    /*
     * Either the dimensions match, or the sequence has length 1 and can
     * be broadcast to the destination.
     */
    if (slen != PyArray_DIMS(a)[dim] && slen != 1) {
        PyErr_Format(PyExc_ValueError,
Repository: numpy/numpy
File: numpy/core/src/multiarray/ctors.c (C)

Code before:
        PyErr_Clear();
        return 0;
    }

    /* obj is a String */
    if (PyString_Check(obj) ||
            PyUnicode_Check(obj)) {
        if (stop_at_string) {
            *maxndim = 0;
        }
        else {
            d[0] = PySequence_Length(obj);
            *maxndim = 1;
        }
        return 0;
    }

    /* obj is a Tuple, but tuples aren't expanded */
    if (stop_at_tuple && PyTuple_Check(obj)) {
        *maxndim = 0;
        return 0;
    }

    /* obj is a PEP 3118 buffer */
    /* PEP 3118 buffer interface */
    if (PyObject_CheckBuffer(obj) == 1) {
        memset(&buffer_view, 0, sizeof(Py_buffer));
        if (PyObject_GetBuffer(obj, &buffer_view,
                    PyBUF_STRIDES|PyBUF_SIMPLE) == 0 ||
                PyObject_GetBuffer(obj, &buffer_view,
                    PyBUF_ND|PyBUF_SIMPLE) == 0) {
            int nd = buffer_view.ndim;

            if (nd < *maxndim) {
                *maxndim = nd;
            }
            for (i = 0; i < *maxndim; i++) {
                d[i] = buffer_view.shape[i];
            }
            PyBuffer_Release(&buffer_view);
            _dealloc_cached_buffer_info(obj);
            return 0;
        }
        else if (PyErr_Occurred()) {
            if (PyErr_ExceptionMatches(PyExc_BufferError) ||
                    PyErr_ExceptionMatches(PyExc_TypeError)) {
                PyErr_Clear();
            } else {
                return -1;
            }
        }

Review comment (suggestion):
Does the removal of this indicate that it gets checked anyway as part of the `_array_from_array_like` code path?

If so, does this fix the dtype inference from a list of ctypes? I'll try and find the issue...

Code after:
                                    discovered_t *out_is_object)
{
    PyObject *e;
    npy_intp n, i;
    PyObject * seq;

    if (*maxndim == 0) {
        return 0;
    }

    /* obj is an Array */
    if (PyArray_Check(obj)) {
        _discover_dimensions_array((PyArrayObject *)obj, maxndim, d);
        return 0;
    }

    /* obj is a Scalar */
    if (PyArray_IsScalar(obj, Generic)) {
        *maxndim = 0;
        return 0;
    }

    /* obj is not a Sequence */
    if (!PySequence_Check(obj) ||
            PySequence_Length(obj) < 0) {
        *maxndim = 0;
        PyErr_Clear();
        return 0;
    }

    /* obj is a String */
    if (PyString_Check(obj) ||
            PyUnicode_Check(obj)) {
        if (stop_at_string) {
            *maxndim = 0;
        }
        else {
            d[0] = PySequence_Length(obj);
            *maxndim = 1;
        }
        return 0;
    }

    /* obj is a Tuple, but tuples aren't expanded */
    if (stop_at_tuple && PyTuple_Check(obj)) {
        *maxndim = 0;
        return 0;
    }

    /*
     * In the future, the result of `_array_from_array_like` should possibly
Repository: numpy/numpy
File: numpy/core/src/multiarray/ctors.c (C)

Code before:

/*
 * Take an arbitrary object and discover how many dimensions it
 * has, filling in the dimensions as we go.
 */
static int
discover_dimensions(PyObject *obj, int *maxndim, npy_intp *d, int check_it,
                                    int stop_at_string, int stop_at_tuple,
                                    discovered_t *out_is_object)
{
    PyObject *e;
    npy_intp n, i;
    Py_buffer buffer_view;
    PyObject * seq;

    if (*maxndim == 0) {
        return 0;
    }

    /* obj is an Array */
    if (PyArray_Check(obj)) {
        PyArrayObject *arr = (PyArrayObject *)obj;

        if (PyArray_NDIM(arr) < *maxndim) {
            *maxndim = PyArray_NDIM(arr);
        }

        for (i=0; i<*maxndim; i++) {
            d[i] = PyArray_DIM(arr,i);
        }
        return 0;
    }

    /* obj is a Scalar */
    if (PyArray_IsScalar(obj, Generic)) {
        *maxndim = 0;
        return 0;
    }

    /* obj is not a Sequence */
    if (!PySequence_Check(obj) ||
            PySequence_Length(obj) < 0) {
        *maxndim = 0;
        PyErr_Clear();
        return 0;
    }

    /* obj is a String */
    if (PyString_Check(obj) ||
            PyUnicode_Check(obj)) {
        if (stop_at_string) {

Review comment (suggestion):
```suggestion
static void
_discover_dimensions_array(PyArrayObject *arr, int *maxndim, npy_intp *d) {
```

Code after:
    for (i = 0; i < n; i++) {
        PyObject *e = PySequence_GetItem(s,i);

        if (e == NULL) {
            return -1;
        }

        r = discover_itemsize(e, nd - 1, itemsize, string_type);
        Py_DECREF(e);
        if (r == -1) {
            return -1;
        }
    }

    return 0;
}


typedef enum {
    DISCOVERED_OK = 0,
    DISCOVERED_RAGGED = 1,
    DISCOVERED_OBJECT = 2
} discovered_t;


static void
_discover_dimensions_array(PyArrayObject *arr, int *maxndim, npy_intp *d) {
    if (PyArray_NDIM(arr) < *maxndim) {
        *maxndim = PyArray_NDIM(arr);
    }
    for (int i = 0; i < *maxndim; i++) {
        d[i] = PyArray_DIM(arr, i);
    }
}


/*
 * Take an arbitrary object and discover how many dimensions it
 * has, filling in the dimensions as we go.
 */
static int
discover_dimensions(PyObject *obj, int *maxndim, npy_intp *d, int check_it,
                                    int stop_at_string, int stop_at_tuple,
                                    discovered_t *out_is_object)
{
    PyObject *e;
    npy_intp n, i;
    PyObject * seq;

    if (*maxndim == 0) {
        return 0;
Repository: numpy/numpy
File: benchmarks/benchmarks/bench_linalg.py (Python)

Code before:
    # some interesting points selected from
    # https://github.com/numpy/numpy/pull/23752#issuecomment-2629521597
    # (m, p, n, batch_size)
    args = [
        (2, 2, 2, 1), (2, 2, 2, 10), (5, 5, 5, 1), (5, 5, 5, 10),
        (10, 10, 10, 1), (10, 10, 10, 10), (20, 20, 20, 1), (20, 20, 20, 10),
        (50, 50, 50, 1), (50, 50, 50, 10),
        (150, 150, 100, 1), (150, 150, 100, 10),
        (400, 400, 100, 1), (400, 400, 100, 10)
    ]

    param_names = ['configuration']

    def __init__(self):
        args = self.args

        a1raw_a2 = [[np.random.rand(batch_size * m * 2 * n).reshape(
            (batch_size, m, 2 * n)
        ), np.random.rand(batch_size * n * p).reshape(
            (batch_size, n, p)
        )] for [m, p, n, batch_size] in args]

        self.args_map = dict([[
            'matmul_' + 'm%03d_p%03d_n%03d_bs%02d' % args[i],
            [a1raw_a2[i][0][:, :, ::2], a1raw_a2[i][1]]
        ] for i in range(len(self.args))])

        self.params = [list(self.args_map.keys())]
    
    def setup(self, configuration):
        self.a1, self.a2 = self.args_map[configuration]

    def time_matmul(self, configuration):
        return np.matmul(self.a1, self.a2)


Review comment (suggestion):
```suggestion
        self.args_map = {
            'matmul_' + 'm%03d_p%03d_n%03d_bs%02d' % args[i]:
                [a1raw_a2[i][0][:, :, ::2], a1raw_a2[i][1]]
            for i in range(len(self.args))})
```
or somethingi like this, I likey misformatted something.

Code after:
    # some interesting points selected from
    # https://github.com/numpy/numpy/pull/23752#issuecomment-2629521597
    # (m, p, n, batch_size)
    args = [
        (2, 2, 2, 1), (2, 2, 2, 10), (5, 5, 5, 1), (5, 5, 5, 10),
        (10, 10, 10, 1), (10, 10, 10, 10), (20, 20, 20, 1), (20, 20, 20, 10),
        (50, 50, 50, 1), (50, 50, 50, 10),
        (150, 150, 100, 1), (150, 150, 100, 10),
        (400, 400, 100, 1), (400, 400, 100, 10)
    ]

    param_names = ['configuration']

    def __init__(self):
        self.args_map = {
            'matmul_m%03d_p%03d_n%03d_bs%02d' % arg: arg for arg in self.args
        }

        self.params = [list(self.args_map.keys())]
    
    def setup(self, configuration):
        m, p, n, batch_size = self.args_map[configuration]

        self.a1raw = np.random.rand(batch_size * m * 2 * n).reshape(
            (batch_size, m, 2 * n)
        )
        
        self.a1 = self.a1raw[:, :, ::2]

        self.a2 = np.random.rand(batch_size * n * p).reshape(
            (batch_size, n, p)
        )

    def time_matmul(self, configuration):
        return np.matmul(self.a1, self.a2)
Repository: numpy/numpy
File: numpy/_core/src/multiarray/unique.cpp (C++)

Code before:
    if (unique_funcs.find(type) == unique_funcs.end()) {
        return Py_None;
    }

    res = unique_funcs[type](self);
    return res;
}



// The following is to expose the unique function to Python
static PyMethodDef UniqueMethods[] = {
    {"unique_hash",  PyArray_Unique, METH_VARARGS,
     "Collect unique values via a hash map."},
    {NULL, NULL, 0, NULL}
};

static struct PyModuleDef uniquemodule = {
    PyModuleDef_HEAD_INIT,
    "_unique", /* name of module */
    NULL, /* module docs */
    -1,  /* size of per-interpreter state of the module,
            or -1 if the module keeps state in global variables. */
    UniqueMethods
};

PyMODINIT_FUNC
PyInit__unique(void)
{
    PyArray_ImportNumPyAPI();
    return PyModule_Create(&uniquemodule);
}


Review comment (refactor):
I feel like we should be able to just include this into `multiarray.c` and add the private helper function there (avoiding the additional module)?

Code after:
{
    if (!PyArray_Check(arr_obj)) {
        PyErr_SetString(PyExc_TypeError,
                "_unique_hash() requires a NumPy array input.");
        return NULL;
    }
    PyArrayObject *arr = (PyArrayObject *)arr_obj;

    try {
        auto type = PyArray_TYPE(arr);
        // we only support data types present in our unique_funcs map
        if (unique_funcs.find(type) == unique_funcs.end()) {
            Py_RETURN_NOTIMPLEMENTED;
        }

        return unique_funcs[type](arr);
    }
    catch (const std::bad_alloc &e) {
        PyErr_NoMemory();
        return NULL;
    }
    catch (const std::exception &e) {
        PyErr_SetString(PyExc_RuntimeError, e.what());
        return NULL;
    }
}
Repository: numpy/numpy
File: numpy/_core/src/multiarray/unique.cpp (C++)

Code before:
    /* This is called from Python space, and expects a single numpy array as input.

    It then returns a numpy array containing the unique values of the input array.

    If the input array is not supported, it returns None.
    */
    // this is to allow grabbing the GIL before raising a python exception.
    NPY_ALLOW_C_API_DEF;


    PyArrayObject *self = NULL;
    PyObject *res = NULL;
    if (!PyArg_ParseTuple(args, "O&", PyArray_Converter, &self))
        return NULL;
    // Making sure the DECREF is called when the function returns, with
    // or w/o an exception
    auto self_decref = finally([&]() { Py_XDECREF(self); });

    try {
        /* Handle zero-sized arrays specially */
        if (PyArray_SIZE(self) == 0) {
            return PyArray_NewLikeArray(
                self,
                NPY_ANYORDER,
                NULL, // descr (use prototype's descr)
                1 // subok
            );
        }

        auto type = PyArray_TYPE(self);
        // we only support data types present in our unique_funcs map
        if (unique_funcs.find(type) == unique_funcs.end()) {
            // Grab GIL before raising an exception
            NPY_ALLOW_C_API;
            PyErr_SetString(PyExc_NotImplementedError, "Unsupported dtype");
            NPY_DISABLE_C_API;
            return NULL;
        }

        res = unique_funcs[type](self);
        return res;
    }
    catch (const std::bad_alloc &e) {
        // Grab GIL before raising an exception
        NPY_ALLOW_C_API;
        PyErr_NoMemory();
        NPY_DISABLE_C_API;
        return NULL;
    }
    catch (const std::exception &e) {
        // Grab GIL before raising an exception

Review comment (suggestion):
```suggestion
                0 // subok (function always returns base-class)
```
I don't think it matters, but noticed the discrepancy.

Code after:
};


/**
 * Python exposed implementation of `_unique_hash`.
 *
 * This is a C only function wrapping code that may cause C++ exceptions into
 * try/catch.
 *
 * @param arr NumPy array to find the unique values of.
 * @return Base-class NumPy array with unique values, `NotImplemented` if the
 * type is unsupported or `NULL` with an error set.
 */
extern "C" NPY_NO_EXPORT PyObject *
array__unique_hash(PyObject *NPY_UNUSED(module), PyObject *arr_obj)
{
    if (!PyArray_Check(arr_obj)) {
        PyErr_SetString(PyExc_TypeError,
                "_unique_hash() requires a NumPy array input.");
        return NULL;
    }
    PyArrayObject *arr = (PyArrayObject *)arr_obj;

    try {
        auto type = PyArray_TYPE(arr);
        // we only support data types present in our unique_funcs map
        if (unique_funcs.find(type) == unique_funcs.end()) {
            Py_RETURN_NOTIMPLEMENTED;
        }

        return unique_funcs[type](arr);
    }
    catch (const std::bad_alloc &e) {
        PyErr_NoMemory();
        return NULL;
    }
    catch (const std::exception &e) {
        PyErr_SetString(PyExc_RuntimeError, e.what());
        return NULL;
    }
}