Skip to content
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

Replace in_tail with a faster implementation. #2527

Merged
merged 2 commits into from
Jul 29, 2019

Conversation

ganmacs
Copy link
Member

@ganmacs ganmacs commented Jul 26, 2019

Which issue(s) this PR fixes:
no

What this PR does / why we need it:

slice! is very slow. so I replaced in_tail implementation with slice + freeze.
This change makes FIFO of in_tail about 100x faster in ruby 2.6's simple benchmark.

benchmark script & result: https://gist.github.com/ganmacs/3f517c79881f56547cde305ae7a86b35

Docs Changes:

no

Release Note:

Replace in_tail with faster implementation.

Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
@ganmacs ganmacs requested a review from repeatedly July 26, 2019 10:20
@repeatedly
Copy link
Member

repeatedly commented Jul 26, 2019

Hmm... why slice! is so slow?
slice! is inplace update so memory usage is lower than slice + freeze?

@repeatedly
Copy link
Member

How about reducing method call by passing @lines?

# call side
@fifo.parse_lines(@lines)

# implementation
def parse_lines(lines)
  idx = @buffer.index(@eol)

  until idx.nil?
    @buffer.freeze
    rbuf = @buffer.slice(0, idx + 1)
    @buffer = @buffer.slice(idx + 1, @buffer.size)
    idx = @buffer.index(@eol)
    lines << convert(rbuf)
  end
end

@ganmacs
Copy link
Member Author

ganmacs commented Jul 29, 2019

According to the profiling, slice + freeze version allocate less memory than slice! one.

slice! seems to create a new string object per each call and copy whole value given as an argument.

Signed-off-by: Yuta Iwama <ganmacs@gmail.com>
@ganmacs
Copy link
Member Author

ganmacs commented Jul 29, 2019

How about reducing method call by passing @lines?

looks good. I confirmed this way is faster.
0b17c04

@repeatedly repeatedly merged commit 8fc0eb9 into fluent:master Jul 29, 2019
@repeatedly
Copy link
Member

Good.

@ganmacs ganmacs deleted the use-slice-to-speed-up branch July 29, 2019 06:00
@repeatedly
Copy link
Member

Note: fluentd uses slice! in several code.

% grep -rne 'slice!' lib/fluent/                                                                   (git)-[master]
lib/fluent//plugin_helper/server.rb:753:                @_handler_write_buffer.slice!(0, written_bytes)
lib/fluent//compat/socket_util.rb:85:          @buffer.slice!(0, pos) if pos > 0
lib/fluent//plugin/in_tcp.rb:86:          conn.buffer.slice!(0, pos) if pos > 0
lib/fluent//plugin/in_syslog.rb:195:          buffer.slice!(0, pos) if pos > 0
lib/fluent//plugin/buffer/memory_chunk.rb:51:          @chunk.slice!(@chunk_bytes, @adding_bytes)

repeatedly added a commit that referenced this pull request Jul 29, 2019
Replace in_tail with a faster implementation.
Signed-off-by: Masahiro Nakagawa <repeatedly@gmail.com>
@ganmacs
Copy link
Member Author

ganmacs commented Jul 30, 2019

I will replace them in my spare time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants