-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(event): optimize concurrent write issues for contract events #6526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(event): optimize concurrent write issues for contract events #6526
Conversation
| v.setTriggerName(Trigger.CONTRACTEVENT_TRIGGER_NAME); | ||
| v.setRemoved(isRemove); | ||
| EventPluginLoader.getInstance().postContractEventTrigger(v); | ||
| synchronized (contractLock) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using blockEvent as the lock granularity for fewer concurrent conflicts.
I also suggest locking the scope outside of forEach.
Furthermore, have you considered using copying to avoid locking issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lock is time consuming operations. Copy is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bladehan1 Using blockEvent as the lock granularity doesn’t seem necessary. Otherwise, multiple lock acquisitions would be required each time, which is quite wasteful in terms of time. It could instead be moved outside the for loop, although the loop itself may sometimes be empty. Personally, I think this kind of fine-grained locking doesn’t incur much performance overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@317787106 The object has too many fields, so directly copying the entire object is not recommended. It’s error-prone, and when new fields are added in the future, it’s easy to forget to initialize or assign them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the memory footprint analysis below, I recommend using defensive copying instead of synchronized blocks. This approach eliminates concurrent write issues with manageable memory overhead.
ContractEventTrigger Memory Footprint Analysis
Primitive fields requiring deep copy (~400 bytes):
timeStamp: long (8 bytes)triggerName: String (~50 bytes)uniqueId: String (~64 bytes, tx_id + index)transactionId: String (~64 bytes, hex string)contractAddress: String (~42 bytes, hex address)callerAddress: String (~42 bytes)originAddress: String (~42 bytes)creatorAddress: String (~42 bytes)blockNumber: Long (8 bytes)blockHash: String (~64 bytes)removed: boolean (1 byte)latestSolidifiedBlockNumber: long (8 bytes)
Complex objects suitable for shallow copy (~800-1800 bytes):
logInfo: LogInfo object (~200-500 bytes) - immutable after creationrawData: RawData object (~100-300 bytes) - read-only referenceabi: ABI object (~100-1000 bytes) - shared contract metadata
Memory Impact Assessment
- Per-event overhead: ~400 bytes (deep copy) + object header (~24 bytes) = ~424 bytes
Benefits over synchronized approach:
- ✅ Eliminates thread contention and blocking
- ✅ Provides linear scalability under high concurrency
- ✅ Minimal memory footprint through selective shallow copying
- ✅ Maintains thread safety without performance degradation
This hybrid copy strategy balances memory efficiency with concurrency performance, making it superior to explicit locking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree @317787106 , I recommend implementing a copy() method for the ContractEventTrigger and ContractLogTrigger classes, and using this defensive copy before modification to solve the concurrency issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sunny6889 @0xbigapple Is copying tens of thousands of objects really more efficient than acquiring a single lock? These objects have too many fields, not just a few, and copying operations will cause inconvenience for future maintenance.
What does this PR do?
Optimize concurrent write issues for contract events.
Why are these changes required?
This PR has been tested by:
Follow up
Extra details