use allocation free converstion from byte slice to string #116

florianl · 2022-07-08T08:34:25Z

Signed-off-by: Florian Lehner florian.lehner@elastic.co

Reduce the number of allocations for converting a slice of bytes to a string.
Less allocation results in less work for GC.

elasticmachine · 2022-07-08T08:37:00Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-07-20T11:59:21.336+0000
Duration: 2 min 30 sec

Test stats 🧪

Test	Results
Failed	0
Passed	397
Skipped	40
Total	437

andrewkroh

edit: updated based on 23fbe24

% benchstat before.txt after.txt 
name                                   old time/op    new time/op    delta
pkg:github.com/elastic/go-libaudit/v2/aucoalesce goos:darwin goarch:amd64
CoalesceMessages-10                      3.11µs ± 0%    3.24µs ± 0%   ~     (p=1.000 n=1+1)
pkg:github.com/elastic/go-libaudit/v2/auparse goos:darwin goarch:amd64
ExtractKeyValuePairs/bpf_message-10      7.81µs ± 0%    7.85µs ± 0%   ~     (p=1.000 n=1+1)
ExtractKeyValuePairs/short_message-10    1.27µs ± 0%    1.28µs ± 0%   ~     (p=1.000 n=1+1)
AuditMessageData/syscall-10              21.9µs ± 0%    21.6µs ± 0%   ~     (p=1.000 n=1+1)
AuditMessageData/user_cmd-10             14.9µs ± 0%    15.0µs ± 0%   ~     (p=1.000 n=1+1)
ParseAuditHeader-10                       147ns ± 0%      98ns ± 0%   ~     (p=1.000 n=1+1)
ParseAuditHeaderRegex-10                  545ns ± 0%     551ns ± 0%   ~     (p=1.000 n=1+1)
ParseLogLine-10                           360ns ± 0%     230ns ± 0%   ~     (p=1.000 n=1+1)

name                                   old alloc/op   new alloc/op   delta
pkg:github.com/elastic/go-libaudit/v2/aucoalesce goos:darwin goarch:amd64
CoalesceMessages-10                      2.72kB ± 0%    2.72kB ± 0%   ~     (all equal)
pkg:github.com/elastic/go-libaudit/v2/auparse goos:darwin goarch:amd64
ExtractKeyValuePairs/bpf_message-10      3.08kB ± 0%    3.08kB ± 0%   ~     (all equal)
ExtractKeyValuePairs/short_message-10    1.00kB ± 0%    1.00kB ± 0%   ~     (p=1.000 n=1+1)
AuditMessageData/syscall-10              15.3kB ± 0%    15.3kB ± 0%   ~     (p=1.000 n=1+1)
AuditMessageData/user_cmd-10             3.68kB ± 0%    3.68kB ± 0%   ~     (p=1.000 n=1+1)
ParseAuditHeader-10                       21.0B ± 0%      0.0B        ~     (p=1.000 n=1+1)
ParseAuditHeaderRegex-10                   128B ± 0%      128B ± 0%   ~     (all equal)
ParseLogLine-10                            358B ± 0%      112B ± 0%   ~     (p=1.000 n=1+1)

name                                   old allocs/op  new allocs/op  delta
pkg:github.com/elastic/go-libaudit/v2/aucoalesce goos:darwin goarch:amd64
CoalesceMessages-10                        13.0 ± 0%      13.0 ± 0%   ~     (all equal)
pkg:github.com/elastic/go-libaudit/v2/auparse goos:darwin goarch:amd64
ExtractKeyValuePairs/bpf_message-10        44.0 ± 0%      44.0 ± 0%   ~     (all equal)
ExtractKeyValuePairs/short_message-10      11.0 ± 0%      11.0 ± 0%   ~     (all equal)
AuditMessageData/syscall-10                 130 ± 0%       130 ± 0%   ~     (all equal)
AuditMessageData/user_cmd-10               64.0 ± 0%      64.0 ± 0%   ~     (all equal)
ParseAuditHeader-10                        3.00 ± 0%      0.00        ~     (p=1.000 n=1+1)
ParseAuditHeaderRegex-10                   2.00 ± 0%      2.00 ± 0%   ~     (all equal)
ParseLogLine-10                            4.00 ± 0%      1.00 ± 0%   ~     (p=1.000 n=1+1)

internal/helper.go

efd6

This should really have a regression test.

internal/helper.go

efd6

I'm thinking that the only place where the unsafe conversion is actually needed is in auparse/hex.go and it could be a local function in that file.

cmd/audit/audit.go

reassembler.go

rule/rule.go

florianl · 2022-07-14T06:55:28Z

friendly ping @andrewkroh && @efd6

efd6 · 2022-07-14T07:48:45Z

After a test is added for the new function I am OK with this. It doesn't need to be much, just that the unsafe string of []byte of x is x.

florianl · 2022-07-14T09:57:17Z

Added tests for UnsafeByteSlice2String.

internal/helper_test.go

Signed-off-by: Florian Lehner <florian.lehner@elastic.co> Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

florianl · 2022-07-21T05:56:02Z

friendly ping @andrewkroh && @efd6

I don't have the permissions to merge something. Therefore, please advice on who to continue.

florianl · 2022-07-29T09:50:36Z

friendly ping @andrewkroh && @efd6 && @adriansr

I don't have the permissions to merge something. Therefore, please advice on who to continue.

Reduce allocations when converting bytes to strings for received messages. [elastic#116](elastic/go-libaudit#116)

florianl · 2022-07-29T14:05:00Z

Thanks 🙏

Reduce allocations when converting bytes to strings for received messages. [#116](elastic/go-libaudit#116) (cherry picked from commit 7a469fd)

Reduce allocations when converting bytes to strings for received messages. [elastic#116](elastic/go-libaudit#116)

Reassembler.Push has always made a copy of the provided byte slice. It elastic#116 it was modified to not copy the slice. But this was a breaking change to its behavior and causes bugs for existing users. The typical use of this function is to parse data from AuditClient.Receive which explicitly documents that caller must make a copy of the data if they are going to retain it beyond the next Receive call. Relates elastic#116

Reassembler.Push has always made a copy of the provided byte slice. It #116 it was modified to not copy the slice. But this was a breaking change to its behavior and causes bugs for existing users. The typical use of this function is to parse data from AuditClient.Receive which explicitly documents that caller must make a copy of the data if they are going to retain it beyond the next Receive call. Relates #116

Reduce allocations when converting bytes to strings for received messages. [#116](elastic/go-libaudit#116)

andrewkroh reviewed Jul 8, 2022

View reviewed changes

internal/helper.go Outdated Show resolved Hide resolved

andrewkroh requested a review from a team July 8, 2022 18:27

andrewkroh added the Team:Security-External Integrations Label for the Security External Integrations team label Jul 8, 2022

efd6 reviewed Jul 8, 2022

View reviewed changes

internal/helper.go Outdated Show resolved Hide resolved

efd6 reviewed Jul 10, 2022

View reviewed changes

cmd/audit/audit.go Outdated Show resolved Hide resolved

reassembler.go Outdated Show resolved Hide resolved

rule/rule.go Outdated Show resolved Hide resolved

efd6 reviewed Jul 14, 2022

View reviewed changes

internal/helper_test.go Outdated Show resolved Hide resolved

internal/helper_test.go Outdated Show resolved Hide resolved

andrewkroh approved these changes Jul 19, 2022

View reviewed changes

florianl and others added 2 commits July 20, 2022 13:58

use allocation free converstion from byte slice to string

4ab06ca

Signed-off-by: Florian Lehner <florian.lehner@elastic.co> Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>

auparse: use string instead of []byte in parseAuditHeader

23fbe24

Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

andrewkroh merged commit 1a53ce9 into elastic:main Jul 29, 2022

andrewkroh added a commit that referenced this pull request Jul 29, 2022

Add changelog entry for #116

f8f7d5c

andrewkroh added a commit to andrewkroh/beats that referenced this pull request Jul 29, 2022

Upgrade go-libaudit@f8f7d5c

c3473bf

Reduce allocations when converting bytes to strings for received messages. [elastic#116](elastic/go-libaudit#116)

andrewkroh mentioned this pull request Jul 29, 2022

[Auditbeat] Upgrade go-libaudit@f8f7d5c elastic/beats#32551

Merged

mergify bot pushed a commit to elastic/beats that referenced this pull request Aug 5, 2022

Upgrade go-libaudit@f8f7d5c (#32551)

12c6631

Reduce allocations when converting bytes to strings for received messages. [#116](elastic/go-libaudit#116) (cherry picked from commit 7a469fd)

bearer-pipeline-test pushed a commit to BearerPipelineTest/beats that referenced this pull request Aug 5, 2022

Upgrade go-libaudit@f8f7d5c (elastic#32551)

7a469fd

Reduce allocations when converting bytes to strings for received messages. [elastic#116](elastic/go-libaudit#116)

andrewkroh mentioned this pull request Aug 24, 2022

[auditbeat][8.4.0] Auditbeat frequently crashing, providing bad data, after upgrade from 8.3.3 to 8.4.0 elastic/beats#32818

Closed

andrewkroh mentioned this pull request Aug 24, 2022

Revert change to Reassembler.Push from #116 #122

Merged

v1v mentioned this pull request Aug 25, 2022

ci: add micro-benchmark #123

Merged

v1v mentioned this pull request Sep 5, 2022

ci: enable report benchmark diff on a PR basis #124

Merged

chrisberkhout pushed a commit to elastic/beats that referenced this pull request Jun 1, 2023

Upgrade go-libaudit@f8f7d5c (#32551)

170c043

Reduce allocations when converting bytes to strings for received messages. [#116](elastic/go-libaudit#116)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use allocation free converstion from byte slice to string #116

use allocation free converstion from byte slice to string #116

florianl commented Jul 8, 2022 •

edited

elasticmachine commented Jul 8, 2022 •

edited

Build stats

Test stats 🧪

andrewkroh left a comment •

edited

efd6 left a comment

efd6 left a comment

florianl commented Jul 14, 2022

efd6 commented Jul 14, 2022

florianl commented Jul 14, 2022

florianl commented Jul 21, 2022

florianl commented Jul 29, 2022

florianl commented Jul 29, 2022

use allocation free converstion from byte slice to string #116

use allocation free converstion from byte slice to string #116

Conversation

florianl commented Jul 8, 2022 • edited

elasticmachine commented Jul 8, 2022 • edited

💚 Build Succeeded

Build stats

Test stats 🧪

andrewkroh left a comment • edited

Choose a reason for hiding this comment

efd6 left a comment

Choose a reason for hiding this comment

efd6 left a comment

Choose a reason for hiding this comment

florianl commented Jul 14, 2022

efd6 commented Jul 14, 2022

florianl commented Jul 14, 2022

florianl commented Jul 21, 2022

florianl commented Jul 29, 2022

florianl commented Jul 29, 2022

florianl commented Jul 8, 2022 •

edited

elasticmachine commented Jul 8, 2022 •

edited

andrewkroh left a comment •

edited