Skip to content

Commit

Permalink
Refactor storage and simplify DocumentMarkerController
Browse files Browse the repository at this point in the history
DocumentMarkerController was storing markers (one of 8 different types)
for text nodes as a Heap<Node, Vector<Vector>>. This has some
disadvantages in practice:
- It wastes memory in the Vector when most nodes only have markers of
  one or two types, using an 8-element vector regardless.
- Almost all queries on markers request only a single type of marker,
  requiring iteration through the map of nodes, touching nodes that might
  have other marker types but not the one of interest.
- The code for managing lifetimes and accessing markers is complex.

This CL changes the layout of marker data to be Vector<Heap<Node, Vector>>,
with a vector of heaps, one per marker type, with each heap entry a node
and it's marker list of that type. This change is motivated by a need to
efficiently find all markers of a particular type as a common
access path, while still being efficient for queries for markers
of a particular type and node, and being no worse than before for
other queries. The code is simpler too, with many fewer checks for
empty marker lists and the like.

The code now correctly enforces the invariant that only nodes with markers appear in the maps.

There is no functional change beyond the ordering of markers returned
to test methods.

Bug: 1443428
Change-Id: I8ab975c32a65dcb76f61ff709bd10891e5b7fefd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4546067
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146457}
  • Loading branch information
schenney-chromium authored and Chromium LUCI CQ committed May 19, 2023
1 parent 585c836 commit 339c81d
Show file tree
Hide file tree
Showing 2 changed files with 370 additions and 341 deletions.

0 comments on commit 339c81d

Please sign in to comment.