Skip to content

Three read-only substitutions for repeated star/point-location queries - #9591

Open
IasonManolas wants to merge 3 commits into
CGAL:mainfrom
IasonManolas:mr/c04-c05-c06
Open

Three read-only substitutions for repeated star/point-location queries#9591
IasonManolas wants to merge 3 commits into
CGAL:mainfrom
IasonManolas:mr/c04-c05-c06

Conversation

@IasonManolas

Copy link
Copy Markdown
Member

Summary

Three independent optimizations, grouped together because each is a small, self-contained
substitution of a cheaper read for a query that a nearby invariant already answers:

  • Read the subdomain of an edge midpoint off the edge. midpoint_with_info() located a
    non-boundary edge's midpoint with subdomain_index_at_point_3(), a full point-location query,
    purely to learn its subdomain. Every cell incident to a non-boundary edge shares one subdomain,
    since is_boundary() is exactly the test that rules out a facet of the complex or a subdomain
    change around the edge. The midpoint lies on the edge, so a locate can only return one of those
    incident cells, and e.first already is one. Reading e.first->subdomain_index() gives the
    same answer without the query. The locate is kept as a CGAL_expensive_assertion.
  • Look for the surface patch without collecting the star. surface_patch_index() collected a
    vertex's entire incident-facet star into a small_vector, then scanned it for the first facet
    belonging to the complex. Streaming through a function output iterator instead removes the
    collection and stops at the first match, same as the scan already did.
  • Examine each neighbour once when validating lengths. are_edge_lengths_valid() visits the
    incident edges of both endpoints of a collapsed edge, so a vertex adjacent to both is visited
    twice; the squared length was recomputed and re-queried from the sizing field on the repeat
    even though the loop's own test reads nothing but the vertex handle. Tracking already-examined
    vertices in a flat_set and skipping repeats removes the redundant sizing-field query.

Testing

Verdict equivalence. All three changes are read-only substitutions or streaming equivalents
over predicates already determined by the triangulation's invariants; none change which edge,
vertex, or collapse is selected, or in what order. Full test suite (all executables in
test/Tetrahedral_remeshing) passes.

Byte-identical output, against plain cgal/main:

mesh tets byte-identical
bear ~40K YES
bunny00 ~225K YES
101556.mesh (Thingi10K) 3,521,237 YES

Performance. perf stat -e instructions, Linux, Release, sequential, against plain
cgal/main.

mesh iterations cgal/main instr this MR Δ instr
bear 10 67.04 G 58.35 G -12.97%
bunny00 10 148.80 G 130.91 G -12.02%
101556.mesh 3 2441.29 G 2263.33 G -7.29%

Release Management

  • Affected package(s): Tetrahedral_remeshing
  • License and copyright ownership: unchanged

… edge

A non-boundary edge's incident cells all share one subdomain, so a
point-location query of its midpoint is redundant: e.first already
carries the answer. The locate is kept as an expensive assertion.
…the star

surface_patch_index() collected a vertex's whole incident-facet star
into a small_vector before scanning it for the first complex facet.
Streaming through incident_facets() with a function output iterator
removes the collection; the scan stops doing work at the first match
the same way the old loop's break did.
…ngths

are_edge_lengths_valid() visits the incident edges of both endpoints
of a collapsed edge, so a vertex adjacent to both is visited twice;
the sizing field was re-queried on the repeat even though the test
reads nothing but the vertex handle. Tracking already-examined
vertices in a flat_set and skipping repeats removes the redundant
query.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants