Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid rebuilding tuple every iteration in GeomBuilder #1266

Merged
merged 6 commits into from
Jun 19, 2024

Conversation

groutr
Copy link
Contributor

@groutr groutr commented May 12, 2023

Construct the set of values for PolyhedralSurface, TIN, and triangle once and reuse to test for those values.

This tuple of values was constructed every iteration of the loop involving several module and object lookups. Now the generated cython is a simple set lookup with an integer.

@groutr
Copy link
Contributor Author

groutr commented May 17, 2023

Original Cython generated code (this is executed for each iteration):

     /* "fiona/_geometry.pyx":135
 *             part = OGR_G_GetGeometryRef(geom, j)
 *             code = base_geometry_type_code(OGR_G_GetGeometryType(part))
 *             if code in (             # <<<<<<<<<<<<<<
 *                 OGRGeometryType.PolyhedralSurface.value,
 *                 OGRGeometryType.TIN.value,
 */
    __pyx_t_3 = __pyx_v_code;
    __pyx_t_2 = __Pyx_PyInt_From_int(__pyx_t_3); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_2);

    /* "fiona/_geometry.pyx":136
 *             code = base_geometry_type_code(OGR_G_GetGeometryType(part))
 *             if code in (
 *                 OGRGeometryType.PolyhedralSurface.value,             # <<<<<<<<<<<<<<
 *                 OGRGeometryType.TIN.value,
 *                 OGRGeometryType.Triangle.value,
 */
    __Pyx_GetModuleGlobalName(__pyx_t_4, __pyx_n_s_OGRGeometryType); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 136, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_4);
    __pyx_t_5 = __Pyx_PyObject_GetAttrStr(__pyx_t_4, __pyx_n_s_PolyhedralSurface); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 136, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_5);
    __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
    __pyx_t_4 = __Pyx_PyObject_GetAttrStr(__pyx_t_5, __pyx_n_s_value); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 136, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_4);
    __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
    __pyx_t_5 = PyObject_RichCompare(__pyx_t_2, __pyx_t_4, Py_EQ); __Pyx_XGOTREF(__pyx_t_5); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;
    __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;

    /* "fiona/_geometry.pyx":135
 *             part = OGR_G_GetGeometryRef(geom, j)
 *             code = base_geometry_type_code(OGR_G_GetGeometryType(part))
 *             if code in (             # <<<<<<<<<<<<<<
 *                 OGRGeometryType.PolyhedralSurface.value,
 *                 OGRGeometryType.TIN.value,
 */
    __pyx_t_6 = __Pyx_PyObject_IsTrue(__pyx_t_5); if (unlikely(__pyx_t_6 < 0)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
    if (!__pyx_t_6) {
    } else {
      __pyx_t_1 = __pyx_t_6;
      goto __pyx_L7_bool_binop_done;
    }
    __pyx_t_5 = __Pyx_PyInt_From_int(__pyx_t_3); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_5);

    /* "fiona/_geometry.pyx":137
 *             if code in (
 *                 OGRGeometryType.PolyhedralSurface.value,
 *                 OGRGeometryType.TIN.value,             # <<<<<<<<<<<<<<
 *                 OGRGeometryType.Triangle.value,
 *             ):
 */
    __Pyx_GetModuleGlobalName(__pyx_t_4, __pyx_n_s_OGRGeometryType); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 137, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_4);
    __pyx_t_2 = __Pyx_PyObject_GetAttrStr(__pyx_t_4, __pyx_n_s_TIN); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 137, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_2);
    __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
    __pyx_t_4 = __Pyx_PyObject_GetAttrStr(__pyx_t_2, __pyx_n_s_value); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 137, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_4);
    __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;
    __pyx_t_2 = PyObject_RichCompare(__pyx_t_5, __pyx_t_4, Py_EQ); __Pyx_XGOTREF(__pyx_t_2); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
    __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;

    /* "fiona/_geometry.pyx":135
 *             part = OGR_G_GetGeometryRef(geom, j)
 *             code = base_geometry_type_code(OGR_G_GetGeometryType(part))
 *             if code in (             # <<<<<<<<<<<<<<
 *                 OGRGeometryType.PolyhedralSurface.value,
 *                 OGRGeometryType.TIN.value,
 */
    __pyx_t_6 = __Pyx_PyObject_IsTrue(__pyx_t_2); if (unlikely(__pyx_t_6 < 0)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;
    if (!__pyx_t_6) {
    } else {
      __pyx_t_1 = __pyx_t_6;
      goto __pyx_L7_bool_binop_done;
    }
    __pyx_t_2 = __Pyx_PyInt_From_int(__pyx_t_3); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_2);

    /* "fiona/_geometry.pyx":138
 *                 OGRGeometryType.PolyhedralSurface.value,
 *                 OGRGeometryType.TIN.value,
 *                 OGRGeometryType.Triangle.value,             # <<<<<<<<<<<<<<
 *             ):
 *                 OGR_G_RemoveGeometry(geom, j, False)
 */
    __Pyx_GetModuleGlobalName(__pyx_t_4, __pyx_n_s_OGRGeometryType); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 138, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_4);
    __pyx_t_5 = __Pyx_PyObject_GetAttrStr(__pyx_t_4, __pyx_n_s_Triangle); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 138, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_5);
    __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
    __pyx_t_4 = __Pyx_PyObject_GetAttrStr(__pyx_t_5, __pyx_n_s_value); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 138, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_4);
    __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
    __pyx_t_5 = PyObject_RichCompare(__pyx_t_2, __pyx_t_4, Py_EQ); __Pyx_XGOTREF(__pyx_t_5); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;
    __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;

    /* "fiona/_geometry.pyx":135
 *             part = OGR_G_GetGeometryRef(geom, j)
 *             code = base_geometry_type_code(OGR_G_GetGeometryType(part))
 *             if code in (             # <<<<<<<<<<<<<<
 *                 OGRGeometryType.PolyhedralSurface.value,
 *                 OGRGeometryType.TIN.value,
 */
    __pyx_t_6 = __Pyx_PyObject_IsTrue(__pyx_t_5); if (unlikely(__pyx_t_6 < 0)) __PYX_ERR(0, 135, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
    __pyx_t_1 = __pyx_t_6;
    __pyx_L7_bool_binop_done:;
    __pyx_t_6 = (__pyx_t_1 != 0);

New cython generated code:


    /* "fiona/_geometry.pyx":152
 *             part = OGR_G_GetGeometryRef(geom, j)
 *             code = base_geometry_type_code(OGR_G_GetGeometryType(part))
 *             if code in PS_TIN_Tri_TYPES:             # <<<<<<<<<<<<<<
 *                 OGR_G_RemoveGeometry(geom, j, False)
 *                 # Removing a geometry will cause the geometry count to drop by one,
 */
    __pyx_t_2 = __Pyx_PyInt_From_int(__pyx_v_code); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 152, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_2);
    if (unlikely(__pyx_v_5fiona_9_geometry_PS_TIN_Tri_TYPES == Py_None)) {
      PyErr_SetString(PyExc_TypeError, "'NoneType' object is not iterable");
      __PYX_ERR(0, 152, __pyx_L1_error)
    }
    __pyx_t_1 = (__Pyx_PySet_ContainsTF(__pyx_t_2, __pyx_v_5fiona_9_geometry_PS_TIN_Tri_TYPES, Py_EQ)); if (unlikely(__pyx_t_1 < 0)) __PYX_ERR(0, 152, __pyx_L1_error)
    __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;
    __pyx_t_3 = (__pyx_t_1 != 0);

sgillies
sgillies previously approved these changes Jun 19, 2024
Copy link
Member

@sgillies sgillies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @groutr !

@sgillies sgillies merged commit cd70508 into Toblerity:main Jun 19, 2024
9 checks passed
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.

2 participants