pit-cs: Remove extra goroutine and Mutex

[zjkmxy]

Ref: #38

[pulsejet]

This deadlocks for some reason, but only when I have multiple faces/producers. Any ideas why?

[zjkmxy]

I have no idea. Could you show me how you run it?

[pulsejet]

Just running two ndnputchunks on different prefixes using a 1G file. Then two ndncatchunks in parallel for both the files.

[zjkmxy]

Thanks. Let me have a try.

[zjkmxy]

Unfortunately I cannot reproduce this on my computer.

One thing you can try is to add a buffer to the channel:

pitCs.updateTimer = make(chan struct{}, 5)

and see if that solves the problem.

If the problem is still there, you could add a core.LogDebug to Update() and AfterFunc, and send the log to me.

[zjkmxy]

Another possible reason could be that there are too many expiring entries that blocks the channel expiringPitEntries.
I cannot reproduce it because my Mac is slow and this case never happens.
Let me try to fix this.

[zjkmxy]

Hello. I pushed another commit. Please see if it fixes the problem.

[pulsejet]

@zjkmxy doesn't lock up for me anymore, but I'm trying to figure out why performance is 25-50% worse. Right now it seems to me that map deletes are simply slow, so doing them on the same thread makes it slower.

But that's unrelated to this PR, so this looks good to me.

[zjkmxy]

@zjkmxy doesn't lock up for me anymore, but I'm trying to figure out why performance is 25-50% worse. Right now it seems to me that map deletes are simply slow, so doing them on the same thread makes it slower.

But that's unrelated to this PR, so this looks good to me.

OK. Let me see if there is solution.
BTW, I guess ndnputchunks does not provide PIT Token? It seems that PITToken is not used when I run the scenario.

[pulsejet]

Right ... maybe I need to stop testing with ndnputchunks

What I currently observe is that if I comment the following line then the aggregate throughput with two instances of putchunks-catchunks, the throughput (content store off) goes from ~3gbps to ~4.5gbps
https://github.com/named-data/YaNFD/blob/79b7e56d00715a9dc8f447ba68dfaec5dfc2f155/table/pit-cs-tree.go#L333

Though I realise now that this could simply be because of the increased GC overhead ...

[zjkmxy]

Right ... maybe I need to stop testing with ndnputchunks

Another possibility is YaNFD does not use PIT Token due to bugs. I'm not sure if this is the case.

What I currently observe is that if I comment the following line then the aggregate throughput with two instances of putchunks-catchunks, the throughput (content store off) goes from ~3gbps to ~4.5gbps

https://github.com/named-data/YaNFD/blob/79b7e56d00715a9dc8f447ba68dfaec5dfc2f155/table/pit-cs-tree.go#L333

Though I realise now that this could simply be because of the increased GC overhead ...

Then, probably using a memory pool for PIT-CS table could be a better choice.
Also, I notice
https://github.com/named-data/YaNFD/blob/79b7e56d00715a9dc8f447ba68dfaec5dfc2f155/table/pit-cs-tree.go#L25
This is wrong. One should use linear data structure. If we have a memory pool for PIT-CS entries, we can simply use the node id.

The implementation of csMap
https://github.com/named-data/YaNFD/blob/79b7e56d00715a9dc8f447ba68dfaec5dfc2f155/table/pit-cs-tree.go#L29
Also does not look good to me. I don't fully understand why it is used. And
https://github.com/named-data/YaNFD/blob/79b7e56d00715a9dc8f447ba68dfaec5dfc2f155/table/pit-cs-tree.go#L372
This seems to assume there is no hash conflict, which could be wrong.

[zjkmxy]

Since this PR is closed. Let's continue further discussion (if there is) on #42.