-
Notifications
You must be signed in to change notification settings - Fork 90
Description
The request handler excessively uses logrus.WithFields()
call to collect entries for the debug logger.
Afaik, each WithFields() call creates a new *logrus.Entry, duplicating the existing underlying map, which is very expensive, especially in a high-frequency code path like this. The built object is not even used if the debug logging is disabled.
These changes were introduced in PR#281
I noticed this when doing continuous memory benchmarks in the Frouter environment. Before the change was introduced, the total allocated memory for 1M requests was under 1GB
│ │ Measured │ Threshold │
├────────────────┼────────────────┼────────────────┤
│ Total alloc │ 901.94 MB │ 1.50 GB
In v0.12:
│ │ Measured │ Threshold │
├────────────────┼────────────────┼────────────────┤
│ Total alloc │ 11.14 GB │ 1.50 GB │
I pushed a draft PR that shows a possible solution by building logrus entry only when needed (it contains a lot of duplicated code to take it as an example). The change at least ensures that the latest GoVPP meets the memory thresholds set in v0.11.