Skip to content

Commit 64b4209

Browse files
ntamasntamas
committed
fix: speed up VertexSeq.select() and EdgeSeq.select() for the whole-graph case, fixes performance problem outlined in #494 
1 parent 8994aa6 commit 64b4209

2 files changed

Lines changed: 195 additions & 98 deletions

File tree

‎src/_igraph/edgeseqobject.c‎

Lines changed: 95 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -331,15 +331,19 @@ PyObject* igraphmodule_EdgeSeq_get_attribute_values_mapping(igraphmodule_EdgeSeq
331331
}
332332

333333
/* Handle strings according to the mapping protocol */
334-
if (PyBaseString_Check(o))
334+
if (PyBaseString_Check(o)) {
335335
return igraphmodule_EdgeSeq_get_attribute_values(self, o);
336+
}
336337

337338
/* Handle iterables and slices by calling the select() method */
338339
if (PySlice_Check(o) || PyObject_HasAttrString(o, "__iter__")) {
339340
PyObject *result, *args;
340341
args = Py_BuildValue("(O)", o);
341-
if (!args)
342+
343+
if (!args) {
342344
return NULL;
345+
}
346+
343347
result = igraphmodule_EdgeSeq_select(self, args);
344348
Py_DECREF(args);
345349
return result;
@@ -559,23 +563,25 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
559563
igraphmodule_EdgeSeqObject *result;
560564
igraphmodule_GraphObject *gr;
561565
igraph_integer_t igraph_idx;
566+
igraph_bool_t working_on_whole_graph = igraph_es_is_all(&self->es);
567+
igraph_vector_t v, v2;
562568
long i, j, n, m;
563569

564-
gr=self->gref;
565-
result=igraphmodule_EdgeSeq_copy(self);
570+
gr = self->gref;
571+
result = igraphmodule_EdgeSeq_copy(self);
566572
if (result == 0)
567573
return NULL;
568574

569575
/* First, filter by positional arguments */
570576
n = PyTuple_Size(args);
571-
for (i=0; i<n; i++) {
577+
for (i = 0; i < n; i++) {
572578
PyObject *item = PyTuple_GET_ITEM(args, i);
573579
if (item == Py_None) {
574580
/* None means: select nothing */
575581
igraph_es_destroy(&result->es);
576582
igraph_es_none(&result->es);
577583
/* We can simply bail out here */
578-
return (PyObject*)result;
584+
return (PyObject*) result;
579585
} else if (PyCallable_Check(item)) {
580586
/* Call the callable for every edge in the current sequence to
581587
* determine what's up */
@@ -588,7 +594,7 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
588594
}
589595

590596
m = PySequence_Size((PyObject*)result);
591-
for (j=0; j<m; j++) {
597+
for (j = 0; j < m; j++) {
592598
PyObject *edge = PySequence_GetItem((PyObject*)result, j);
593599
PyObject *call_result;
594600
if (edge == 0) {
@@ -602,10 +608,12 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
602608
igraph_vector_destroy(&v);
603609
return NULL;
604610
}
605-
if (PyObject_IsTrue(call_result))
611+
if (PyObject_IsTrue(call_result)) {
606612
igraph_vector_push_back(&v,
607613
igraphmodule_Edge_get_index_long((igraphmodule_EdgeObject*)edge));
608-
else was_excluded=1;
614+
} else {
615+
was_excluded = 1;
616+
}
609617
Py_DECREF(call_result);
610618
Py_DECREF(edge);
611619
}
@@ -627,79 +635,105 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
627635
* to restrict the edge set. Integers are interpreted as indices on the
628636
* edge set and NOT on the original, untouched edge sequence of the
629637
* graph */
630-
igraph_vector_t v, v2;
631638
if (igraph_vector_init(&v, 0)) {
632639
igraphmodule_handle_igraph_error();
633640
return 0;
634641
}
635-
if (igraph_vector_init(&v2, 0)) {
636-
igraph_vector_destroy(&v);
637-
igraphmodule_handle_igraph_error();
638-
return 0;
639-
}
640-
if (igraph_es_as_vector(&gr->g, self->es, &v2)) {
641-
igraph_vector_destroy(&v);
642-
igraph_vector_destroy(&v2);
643-
igraphmodule_handle_igraph_error();
644-
return 0;
642+
643+
if (!working_on_whole_graph) {
644+
/* Extract the current vertex sequence into a vector */
645+
if (igraph_vector_init(&v2, 0)) {
646+
igraph_vector_destroy(&v);
647+
igraphmodule_handle_igraph_error();
648+
return 0;
649+
}
650+
if (igraph_es_as_vector(&gr->g, self->es, &v2)) {
651+
igraph_vector_destroy(&v);
652+
igraph_vector_destroy(&v2);
653+
igraphmodule_handle_igraph_error();
654+
return 0;
655+
}
656+
m = igraph_vector_size(&v2);
657+
} else {
658+
/* v2 left uninitialized, we are not going to use it as it would
659+
* simply contain integers from 0 to ecount(g)-1 */
660+
m = igraph_ecount(&gr->g);
645661
}
646-
m = igraph_vector_size(&v2);
647-
for (; i<n; i++) {
662+
663+
for (; i < n; i++) {
648664
PyObject *item2 = PyTuple_GET_ITEM(args, i);
649665
long idx;
650666
if (!PyLong_Check(item2)) {
651667
Py_DECREF(result);
652668
PyErr_SetString(PyExc_TypeError, "edge indices expected");
653669
igraph_vector_destroy(&v);
654-
igraph_vector_destroy(&v2);
670+
if (!working_on_whole_graph) {
671+
igraph_vector_destroy(&v2);
672+
}
655673
return NULL;
656674
}
657675
idx = PyLong_AsLong(item2);
658676
if (idx >= m || idx < 0) {
659677
PyErr_SetString(PyExc_ValueError, "edge index out of range");
660678
igraph_vector_destroy(&v);
661-
igraph_vector_destroy(&v2);
679+
if (!working_on_whole_graph) {
680+
igraph_vector_destroy(&v2);
681+
}
662682
return NULL;
663683
}
664-
if (igraph_vector_push_back(&v, VECTOR(v2)[idx])) {
684+
if (igraph_vector_push_back(&v, working_on_whole_graph ? idx : VECTOR(v2)[idx])) {
665685
Py_DECREF(result);
666686
igraphmodule_handle_igraph_error();
667687
igraph_vector_destroy(&v);
668-
igraph_vector_destroy(&v2);
688+
if (!working_on_whole_graph) {
689+
igraph_vector_destroy(&v2);
690+
}
669691
return NULL;
670692
}
671693
}
672-
igraph_vector_destroy(&v2);
694+
695+
if (!working_on_whole_graph) {
696+
igraph_vector_destroy(&v2);
697+
}
698+
673699
igraph_es_destroy(&result->es);
700+
674701
if (igraph_es_vector_copy(&result->es, &v)) {
675702
Py_DECREF(result);
676703
igraphmodule_handle_igraph_error();
677704
igraph_vector_destroy(&v);
678705
return NULL;
679706
}
707+
680708
igraph_vector_destroy(&v);
681709
} else {
682710
/* Iterators and everything that was not handled directly */
683711
PyObject *iter, *item2;
684-
igraph_vector_t v, v2;
685712

686713
/* Allocate stuff */
687714
if (igraph_vector_init(&v, 0)) {
688715
igraphmodule_handle_igraph_error();
689716
return 0;
690717
}
691-
if (igraph_vector_init(&v2, 0)) {
692-
igraph_vector_destroy(&v);
693-
igraphmodule_handle_igraph_error();
694-
return 0;
695-
}
696-
if (igraph_es_as_vector(&gr->g, self->es, &v2)) {
697-
igraph_vector_destroy(&v);
698-
igraph_vector_destroy(&v2);
699-
igraphmodule_handle_igraph_error();
700-
return 0;
718+
if (!working_on_whole_graph) {
719+
/* Extract the current vertex sequence into a vector */
720+
if (igraph_vector_init(&v2, 0)) {
721+
igraph_vector_destroy(&v);
722+
igraphmodule_handle_igraph_error();
723+
return 0;
724+
}
725+
if (igraph_es_as_vector(&gr->g, self->es, &v2)) {
726+
igraph_vector_destroy(&v);
727+
igraph_vector_destroy(&v2);
728+
igraphmodule_handle_igraph_error();
729+
return 0;
730+
}
731+
m = igraph_vector_size(&v2);
732+
} else {
733+
/* v2 left uninitialized, we are not going to use it as it would
734+
* simply contain integers from 0 to ecount(g)-1 */
735+
m = igraph_ecount(&gr->g);
701736
}
702-
m = igraph_vector_size(&v2);
703737

704738
/* Create an appropriate iterator */
705739
if (PySlice_Check(item)) {
@@ -708,7 +742,7 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
708742
PyObject* range;
709743
igraph_bool_t ok;
710744

711-
ok = (PySlice_GetIndicesEx(item, igraph_vector_size(&v2), &start, &stop, &step, &sl) == 0);
745+
ok = (PySlice_GetIndicesEx(item, m, &start, &stop, &step, &sl) == 0);
712746
if (ok) {
713747
range = igraphmodule_PyRange_create(start, stop, step);
714748
ok = (range != 0);
@@ -720,7 +754,9 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
720754
}
721755
if (!ok) {
722756
igraph_vector_destroy(&v);
723-
igraph_vector_destroy(&v2);
757+
if (!working_on_whole_graph) {
758+
igraph_vector_destroy(&v2);
759+
}
724760
PyErr_SetString(PyExc_TypeError, "error while converting slice to iterator");
725761
Py_DECREF(result);
726762
return 0;
@@ -733,13 +769,15 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
733769
/* Did we manage to get an iterator? */
734770
if (iter == 0) {
735771
igraph_vector_destroy(&v);
736-
igraph_vector_destroy(&v2);
772+
if (!working_on_whole_graph) {
773+
igraph_vector_destroy(&v2);
774+
}
737775
PyErr_SetString(PyExc_TypeError, "invalid edge filter among positional arguments");
738776
Py_DECREF(result);
739777
return 0;
740778
}
741779
/* Do the iteration */
742-
while ((item2=PyIter_Next(iter)) != 0) {
780+
while ((item2 = PyIter_Next(iter)) != 0) {
743781
if (igraphmodule_PyObject_to_integer_t(item2, &igraph_idx)) {
744782
/* We simply ignore elements that we don't know */
745783
Py_DECREF(item2);
@@ -750,28 +788,38 @@ PyObject* igraphmodule_EdgeSeq_select(igraphmodule_EdgeSeqObject *self, PyObject
750788
Py_DECREF(result);
751789
Py_DECREF(iter);
752790
igraph_vector_destroy(&v);
753-
igraph_vector_destroy(&v2);
791+
if (!working_on_whole_graph) {
792+
igraph_vector_destroy(&v2);
793+
}
754794
return NULL;
755795
}
756-
if (igraph_vector_push_back(&v, VECTOR(v2)[(long int) igraph_idx])) {
796+
if (igraph_vector_push_back(&v, working_on_whole_graph ? igraph_idx : VECTOR(v2)[(long int) igraph_idx])) {
757797
Py_DECREF(result);
758798
Py_DECREF(iter);
759799
igraphmodule_handle_igraph_error();
760800
igraph_vector_destroy(&v);
761-
igraph_vector_destroy(&v2);
801+
if (!working_on_whole_graph) {
802+
igraph_vector_destroy(&v2);
803+
}
762804
return NULL;
763805
}
764806
}
765807
}
808+
766809
/* Deallocate stuff */
767-
igraph_vector_destroy(&v2);
810+
if (!working_on_whole_graph) {
811+
igraph_vector_destroy(&v2);
812+
}
813+
768814
Py_DECREF(iter);
769815
if (PyErr_Occurred()) {
770816
igraph_vector_destroy(&v);
771817
Py_DECREF(result);
772818
return 0;
773819
}
820+
774821
igraph_es_destroy(&result->es);
822+
775823
if (igraph_es_vector_copy(&result->es, &v)) {
776824
Py_DECREF(result);
777825
igraphmodule_handle_igraph_error();

0 commit comments

Comments
 (0)