search

named-data / YaNFD

Yet another Named Data Networking Forwarding Daemon
https://pkg.go.dev/github.com/named-data/YaNFD
MIT License
13 stars 10 forks source link

pit-cs: Remove extra goroutine and Mutex #40

Closed zjkmxy closed 2 years ago

zjkmxy commented 2 years ago

Ref: #38

pulsejet commented 2 years ago

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

zjkmxy commented 2 years ago

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

pulsejet commented 2 years ago

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

zjkmxy commented 2 years ago

Thanks. Let me have a try.

zjkmxy commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

pulsejet commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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