Project

General

Profile

Actions

Feature #3361

open

FIB/PIT/CS/Measurements: erase by iterator

Added by Alex Afanasyev over 8 years ago. Updated about 7 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
Tables
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
6.00 h

Description

In FIB/PIT/CS/Measurements tables, allow erasing an entry by const_iterator.

Suggested by Natalya Rozhnova on ndnsim mailing list.

Effectively, the existing API allows iteration over the table, but an iterator cannot be directly used to remove entries.
There are workarounds, but we need to provide a clean API for this operation.


Related issues 1 (0 open1 closed)

Related to NFD - Task #3164: Reduce usage of shared_ptr in FIB/PIT/StrategyChoice/MeasurementsClosedJunxiao Shi

Actions
Actions #1

Updated by Junxiao Shi over 8 years ago

  • Related to Task #3164: Reduce usage of shared_ptr in FIB/PIT/StrategyChoice/Measurements added
Actions #2

Updated by Junxiao Shi over 8 years ago

  • Tracker changed from Bug to Feature
  • Subject changed from PIT/CS/FIB/Measurements should allow erasing using iterator to FIB/PIT/CS/Measurements: erase by iterator
  • Description updated (diff)
  • Estimated time set to 6.00 h

The original purpose of enumeration API is to inspect the table contents. It's not intended to allow table modification, hence the API takes shared_ptr.
Thus, this is not a Bug but a new Feature.

This can be addressed together with #3164.

Actions #3

Updated by Alex Afanasyev over 8 years ago

I wouldn't say the issues are tightly related. We don't have API to use iterator as a parameter to erase method. I suggest we add this extension asap, before making extensive overhaul with shared pointers (which may have unknown unintended consequences).

Actions #4

Updated by Spyros Mastorakis over 8 years ago

  • Status changed from New to In Progress
  • Assignee set to Spyros Mastorakis
  • % Done changed from 0 to 60

I have done the PIT and FIB part. It is not clear to me, what should be done in the case of CS and the Measurements table. For CS, there is one erase method declaration and the method has not been implemented yet. For the Measurements table, there is a cleanup method (which is private) that erases an entry from the name-tree. I have uploaded a very initial version of the commit on gerrit, so please take a look and comment on it:

http://gerrit.named-data.net/#/c/2663/

Actions #5

Updated by Junxiao Shi over 8 years ago

Don't rush this. We need to look at the design in a bigger picture together with #3164 note-3: should be use iterators as the primary handle of table items, or should we keep using shared_ptrs or references?

Actions #6

Updated by Alex Afanasyev over 8 years ago

#3164 note-3 is a separate issue and has no direct relationship to this iterator API improvement issue.

Actions #7

Updated by Junxiao Shi over 8 years ago

#3164 note-3 is a separate issue and has no direct relationship to this iterator API improvement issue.

Although it's kind of "separate", we must look at the bigger picture before continuing.

Thus, the design issue should be discussed in NFD call before approving this change.

The iterator API is designed to support enumeration only, so there's no erase/insert/etc.

Anything beyond enumeration is a major semantics change, and we must make sure it is consistent with the bigger picture.

If "erase by iterator" is for ndnSIM only, commit this to ndnSIM fork, and I won't care.

Actions #8

Updated by Junxiao Shi almost 8 years ago

I notice https://gerrit.named-data.net/2663 patchset1 declares void erase(const_iterator it).

But the common declaration in STL and Boost is iterator erase(const_iterator it).

Actions #9

Updated by Junxiao Shi about 7 years ago

  • Status changed from In Progress to New
  • Assignee deleted (Spyros Mastorakis)
  • Target version deleted (v0.5)
  • % Done changed from 60 to 0

20170201 call agrees to stop working on this issue until a valid design is found.
The reason we cannot have a proper erase-by-iterator API is: any erasure invalidates all NameTree-attached iterators.

Actions

Also available in: Atom PDF