replaced class with struct in ixy project

[egdeeno]

changed class to struct where possible in ixy project; only class MemoryMap is still class.
Maybe replacing class with struct will solve retain/release problem: "A total of 76% of the CPU time is spent incrementing and decrementing reference counters".
fixme in File.swift: deinit block commented out, so need to close(fd) somewhere else.

used Swift version 5.1-dev (LLVM b47beb8a70, Swift fcc37cda9c)

[LucasVanDongen]

Could you test the actual code or it's just a search / replace with compilation fixes? I don't have access to the actual hardware but replacing classes with structs would be my first guess too. But to get the most out of it I think a more functional approach would be better instead of going for OO-with-structs.

[egdeeno]

Could you test the actual code or it's just a search / replace with compilation fixes? I don't have access to the actual hardware but replacing classes with structs would be my first guess too. But to get the most out of it I think a more functional approach would be better instead of going for OO-with-structs.

it is just search/replace with compilation fixes, because have no hardware to test for real.

[LucasVanDongen]

I think using @inline on some functions might help prevent unneeded allocations as well without sacrificing readability.

[emmericp]

Thank you for your pull request, unfortunately it doesn't work :(

Swift <= 5.0: doesn't compile

Swift 5.1-dev LLVM b47beb8a70, Swift b9d082fac7: doesn't forward packets:

root@narva:~/ixy.swift/app# swift run -c release app forward 0000:01:00.0 0000:03:00.0
[app   ] Starting forward from 0000:01:00.0 to 0000:03:00.0
[driver] Could not unbind: /sys/bus/pci/devices/0000:01:00.0/driver/unbind
[device] Allocating packet buffer for 2048 packets (size=4194304)
[driver] Resetting 0000:01:00.0
[driver] Initializing 0000:01:00.0
[driver] Waiting for link...
[driver] Link speed 10Gbit/s
[device] Device Ready
[driver] Could not unbind: /sys/bus/pci/devices/0000:03:00.0/driver/unbind
[device] Allocating packet buffer for 2048 packets (size=4194304)
[driver] Resetting 0000:03:00.0
[driver] Initializing 0000:03:00.0
[driver] Waiting for link...
[driver] Link speed 10Gbit/s
[device] Device Ready
[    rx] No packet pointer
[    rx] No packet pointer
...

(the Could not unbind message is normal because the device was already used in a run before)

I've also re-run master branch with this Swift 5.1-dev version and it slows down to 3.7 Mpps vs. 5.1 Mpps before

[emmericp]

CC @thomasguenzel

[thomasguenzel]

It's been quite some time since I've written this code so I might be wrong, but I'm guessing that you'd have to rewrite a lot more of the code to support structs.
Structs are – to my knowledge – passed by value, so any time you are handling (for example) a descriptor, a complete copy is created. You can read about it here. This means you don't really change the queues, you change copies of structs in the queue without doing any changes on the queue itself.
That was the reason behind using classes instead of structs. I'm guessing it would be possible to write a solution using structs, but that would require a lot more than just converting the classes to structs. You'd probably have to rewrite at least the whole Queue and Descriptor handling, maybe the complete codebase...

[egdeeno]

Thank you for your pull request, unfortunately it doesn't work :(

Swift <= 5.0: doesn't compile

Swift 5.1-dev LLVM b47beb8a70, Swift b9d082fac7: doesn't forward packets:

[driver] Waiting for link...
[driver] Link speed 10Gbit/s
[device] Device Ready
[    rx] No packet pointer
[    rx] No packet pointer
...

(the Could not unbind message is normal because the device was already used in a run before)

I've also re-run master branch with this Swift 5.1-dev version and it slows down to 3.7 Mpps vs. 5.1 Mpps before

i doubt i've found the problem. There is function withHugepageMemory in Queue, ReceiveQueue, TransmitQueue.
i can pull request, or this is the changed code.
In Queue:
static func withHugepageMemory(index: UInt, packetMempool: DMAMempool, descriptorCount: UInt, driver: Driver) throws -> Queue { let pageSize = (Int(descriptorCount) * MemoryLayout<Int64>.size * 2) let hugepage = try Hugepage(size: pageSize, requireContiguous: true) return try Queue(index: index, memory: hugepage.memoryMap, packetMempool: packetMempool, descriptorCount: descriptorCount, driver: driver) }

in TransmitQueue:
static func withHugepageMemory(index: UInt, packetMempool: DMAMempool, descriptorCount: UInt, driver: Driver) throws -> TransmitQueue { let pageSize = (Int(descriptorCount) * MemoryLayout<Int64>.size * 2) let hugepage = try Hugepage(size: pageSize, requireContiguous: true) return try TransmitQueue(index: index, memory: hugepage.memoryMap, packetMempool: packetMempool, descriptorCount: descriptorCount, driver: driver) }

in ReceiveQueue:
static func withHugepageMemory(index: UInt, packetMempool: DMAMempool, descriptorCount: UInt, driver: Driver) throws -> ReceiveQueue { let pageSize = (Int(descriptorCount) * MemoryLayout<Int64>.size * 2) let hugepage = try Hugepage(size: pageSize, requireContiguous: true) return try ReceiveQueue(index: index, memory: hugepage.memoryMap, packetMempool: packetMempool, descriptorCount: descriptorCount, driver: driver) }

i tried compile the changed code with Swift version 5.0.3 (swift-5.0.3-RELEASE) and it was ok.
Swift version 5.1-dev (LLVM b47beb8a70, Swift fcc37cda9c) also builds.

[egdeeno]

It's been quite some time since I've written this code so I might be wrong, but I'm guessing that you'd have to rewrite a lot more of the code to support structs.
Structs are – to my knowledge – passed by value, so any time you are handling (for example) a descriptor, a complete copy is created. You can read about it here. This means you don't really change the queues, you change copies of structs in the queue without doing any changes on the queue itself.
That was the reason behind using classes instead of structs. I'm guessing it would be possible to write a solution using structs, but that would require a lot more than just converting the classes to structs. You'd probably have to rewrite at least the whole Queue and Descriptor handling, maybe the complete codebase...

yes, i'll try to change

[egdeeno]

i replaced for var with for indx in 0..< in ReceiveQueue and TransmitQueue, so i doubt it is what you mean @thomasguenzel ?
For e.g.:

for indx in txCount..<packets.count {
	packets[indx].free()
	freed += 1
}

[thomasguenzel]

It might work, but as I said: there's more to it than just changing all classes to structs. This probably isn't the only place where structs are copied. You'll have to rework the whole code.
I just opened file (ReceiveQueue) and instantly found a few places which won't work. For another example:

while process(descriptor: &tmpDescriptor) {
	lastIndex = self.queue.tailIndex
	self.queue.tailIndex ++< self.queue.descriptors.count
}

tmpDescriptor is a copy from the descriptors, so processing it won't change the actual queue.
This is just one example of many.

[egdeeno]

It might work, but as I said: there's more to it than just changing all classes to structs. This probably isn't the only place where structs are copied. You'll have to rework the whole code.
I just opened file (ReceiveQueue) and instantly found a few places which won't work. For another example:

while process(descriptor: &tmpDescriptor) {
	lastIndex = self.queue.tailIndex
	self.queue.tailIndex ++< self.queue.descriptors.count
}

tmpDescriptor is a copy from the descriptors, so processing it won't change the actual queue.
This is just one example of many.

yes, i think, it can be changed in this way:
replace cleanUp(descriptor: inout Descriptor) with cleanUp(index: Int)

[akbashev]

It's actually would be interesting to see how is Swift performing without ARC.
But yeah, a lot time passed as I see, there is more than changing directly to structs. (but still think should be doable 🤔)