Skip to content

Commit f2fbeb6

Browse files
simonmarmeta-codesync[bot]
authored andcommitted
Fix an assertion failure (#616)
Summary: `Id::operator-=(a,b)` throws when a == b, because you're not supposed to make an invalid Id from a valid Id. So instead we use `uint64_t` to represent the page offsets, because those are not necesarily valid Ids. This assertion triggered when I tried to add units to the Haskell indexer, I'm not sure why it hasn't triggered before. Pull Request resolved: #616 Reviewed By: jjuliamolin Differential Revision: D89959600 Pulled By: kbojarczuk fbshipit-source-id: 3c93733e92f64dda0ad17f5ef297dd92a9823bd9
1 parent 66ab5ee commit f2fbeb6

1 file changed

Lines changed: 11 additions & 8 deletions

File tree

glean/rts/ownership.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,15 +442,18 @@ FOLLY_NOINLINE void completeOwnership(
442442
auto executor = folly::getGlobalCPUExecutor();
443443

444444
folly::Future<folly::Unit> fetcher = folly::via(executor, [&]() {
445-
auto pageOf = [](Id id) {
446-
return Id::fromWord((id.toWord() / pageSize) * pageSize);
445+
auto pageOf = [](Id id) -> uint64_t {
446+
return (id.toWord() / pageSize) * pageSize;
447447
};
448-
Id last = pageOf(lookup.firstFreeId() - 1);
449-
Id first = pageOf(lookup.startingId());
450-
for (Id id = last;; id -= pageSize) {
451-
VLOG(1) << folly::sformat("fetching page: {}", id.toWord());
452-
queue.blockingWrite(fetchPage(id, id + pageSize));
453-
if (id == first) {
448+
Id start = lookup.startingId();
449+
Id last = lookup.firstFreeId() - 1;
450+
uint64_t first = pageOf(start);
451+
for (uint64_t page = pageOf(last);; page -= pageSize) {
452+
VLOG(1) << folly::sformat("fetching page: {}", page);
453+
queue.blockingWrite(fetchPage(
454+
(page == first) ? start : Id::fromWord(page),
455+
Id::fromWord(page + pageSize)));
456+
if (page == first) {
454457
break;
455458
}
456459
}

0 commit comments

Comments
 (0)