Skip to content

[TimeCache] Improve performance for insertData() and pruneList() (backport #680)#694

Merged
ahcorde merged 3 commits intoironfrom
mergify/bp/iron/pr-680
May 29, 2024
Merged

[TimeCache] Improve performance for insertData() and pruneList() (backport #680)#694
ahcorde merged 3 commits intoironfrom
mergify/bp/iron/pr-680

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented May 28, 2024

Resolves #676

Per #676 (comment), this should yield a ~1000x speedup for insertData (for inserting new data only)


This is an automatic backport of pull request #680 done by Mergify.

@mergify mergify bot requested a review from ahcorde as a code owner May 28, 2024 23:09
@mergify mergify bot added the conflicts label May 28, 2024
@mergify mergify bot requested a review from clalancette as a code owner May 28, 2024 23:09
@mergify
Copy link
Contributor Author

mergify bot commented May 28, 2024

Cherry-pick of d700d78 has failed:

On branch mergify/bp/iron/pr-680
Your branch is up to date with 'origin/iron'.

You are currently cherry-picking commit d700d786.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   tf2/src/cache.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

nachovizzo and others added 2 commits May 29, 2024 10:31
* Remove unused parameter

* Make use of API function to improve redability

```cpp
TimePoint TimeCache::getLatestTimestamp()
{
  return storage_.front().stamp_;
}
```

And std::list<T>::front() is(gcclib):
```cpp
reference
front() _GLIBCXX_NOEXCEPT
{ return *begin(); }
```

* Same argument as 321bd22

```cpp

TimePoint TimeCache::getLatestTimestamp()
{
  // empty list case
  // ...
  return storage_.front().stamp_;
}
```

and std::list<T>::front():
```cpp
reference
front() _GLIBCXX_NOEXCEPT
{ return *begin(); }
```

* Improve readbility by relying on STL functions

By now reading to this block I can tell that we are preventing to
inserting a new element in the list, that has a timestamp that is
actually older than the max_storage_time_ we allow for

* Remove hardcoded algorithmg for STL one

The intent of the code is now more clear, instead of relying on raw
loops, we "find if" there is any element in the list that has a stamp
older than the incoming one. With this we find the position in the list
where we should insert the current timestamp: `storage_it`

* Remove to better express what this pointer is represetngin

* Replace raw loop for STL algorithm

Remove if any element is older thant the max_storage_time_ allowed,
relative to the latest(sooner) time seems clear npw
Signed-off-by: Eric Cousineau <eric.cousineau@tri.global>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
@ahcorde ahcorde force-pushed the mergify/bp/iron/pr-680 branch from 4f1e4c1 to d1143f5 Compare May 29, 2024 08:31
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit 75efe0c into iron May 29, 2024
@delete-merged-branch delete-merged-branch bot deleted the mergify/bp/iron/pr-680 branch May 29, 2024 11:07
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