I have a conceptually simple application: data comes in as small encrypted packets, they decrypted and validated, some are stored in the database, some rules are applied, and a reply might be encrypted and sent. Packets arrive at a rate of high hundreds of thousands per thread per second, so performance is a consideration. I am leaving out most of the performance elements here to focus on the design.
Main elements: packets arrive in a queue and each processor thread gets one at a time:
- take ownership of a single incoming packet (at a time)
- decrypt
- use a factory to construct the appropriate packet class
- packet saves itself to database (often multiple inserts)
- rule engine run by some types of packets, resulting actions queued
- packet may generate 1 or more replies to 1 or more recipients
One problem is that each processor thread owns a bunch of helper instances that get passed into the constructor of the packets. Those helper classes are often painful to create (like the database connection), so I don't want the packet to construct them. During decryption I accumulate information about the packet from the database so I want to pass that in too. That's a lot of parameters.
I've ended up with 15 parameters to the PacketClass constructor and growing. That seems excessive, but I can't see an elegant solution.
The constructor looks like this:
PacketClass(database, cryptoEngine, {more references} ...
packetDataBlob, packetBlobSize, {packet source information},
{database values passed rather than repeating lookups}...
At some stage I intend to cache the database info that gets looked up during decryption because there's only going to be a million or so records and that lookup will be a bottleneck. But for now there's ~50 bytes in 6 values that get looked up for each packet then passed into the constructor from one SELECT query. Once the cache is operational those will become a single const reference but for now they're not.
Options:
- make all the utility class references global or thread-global as per the IOC/service locator trend
- pass a reference to the owning thread/processor to each packet giving quite horrid linking
- wrap the params into a couple of data classes and pretend there's nothing to see.
- split the PacketClass constructor into construct-with-references and initialise-with-data methods (does this even count as a solution?)
- some brilliant solution I haven't thought of
Can anyone suggest options?
Robert Harvey suggested in chat splitting the processing into a PacketProcessorClass for each PacketClass so that behaviour and data are separated. That way much of the processing is done to the PacketClass in the processor thread rather than inside the packet as it is now. I'm going to work through that design over the weekend. Other ideas are also welcome.
New Design:
PacketClass( packetDataBlob, packetBlobSize, {packet source information},
{database values passed rather than repeating lookups}...
PacketClassProcessor(database, cryptoEngine, {more references})
PacketClassProcessor.Process(Packet)
Performance considerations: this might even be faster than the current setup. I expect to be able to cache most of the ProcessorClass instances, so there's not that construction overhead. And the PacketClass could become a C struct so again no constructor overhead except for any collections it contains... and I can cache at least one of those (the "rules that apply to this packet' one).
(I edited out some fluff too)