Project

General

Profile

Actions

Feature #14426

closed

[PATCH] openssl: reduce memory allocation in OpenSSL::Buffering#do_write

Feature #14426: [PATCH] openssl: reduce memory allocation in OpenSSL::Buffering#do_write

Added by janko (Janko Marohnić) over 7 years ago. Updated about 7 years ago.

Status:
Closed
Target version:
-
[ruby-core:85298]

Description

When writing data to an SSLSocket, there are a lot of, in my opinion, unnecessary strings being allocated, concretely in OpenSSL::Buffering#do_write.

When the buffer would be written, it would always be copied into a new string first, regardless of whether the write was partial or not. And in case of partial writes, it's not necessary to create copies of remaining data, we could just use the String[from, length] = "" trick immediately which modifies the string in-place.

I also thought that splitting writes on newlines was adding unnecessary memory allocations, so I removed that.

I tested uploading a 5MB file using HTTP.rb, and memory allocation went from 7.7 MB to 0.2 MB with this change.

require "http"
require "memory_profiler"
require "stringio"

body = StringIO.new("a" * 5*1024*1024)

MemoryProfiler.report do
  HTTP.post("https://example.com", body: body)
end.pretty_print

Files

openssl-memory-allocation.patch (913 Bytes) openssl-memory-allocation.patch janko (Janko Marohnić), 07/07/2018 05:04 PM

Related issues 1 (0 open1 closed)

Related to Ruby - Bug #20972: OpenSSL Memory UsageClosedActions

Updated by normalperson (Eric Wong) over 7 years ago Actions #1 [ruby-core:85315]

wrote:

https://bugs.ruby-lang.org/issues/14426

I also thought that splitting writes on newlines was adding
unnecessary memory allocations, so I removed that.

Agreed.

I tested uploading a 5MB file using HTTP.rb, and memory
allocation went from 7.7 MB to 0.2 MB with this change.

Nice! This patch looks good to me, thanks.

Updated by hsbt (Hiroshi SHIBATA) over 7 years ago Actions #2 [ruby-core:85318]

  • Status changed from Open to Assigned
  • Assignee set to rhenium (Kazuki Yamaguchi)

Updated by janko (Janko Marohnić) over 7 years ago Actions #3

Any updates on this one? I think it would be nice to have this included in the next patch version. This will make memory usage during large file uploads consistent regardless of whether it's going to HTTP or HTTPS endpoint; currently uploads over HTTPS consume a lot more memory, while HTTP uploads don't really allocate extra memory.

Updated by janko (Janko Marohnić) over 7 years ago Actions #4

  • File openssl-memory-allocation.patch added
  • File deleted (openssl-memory-allocation.patch)

Updated by janko (Janko Marohnić) over 7 years ago Actions #5

  • File deleted (openssl-memory-allocation.patch)

Updated by janko (Janko Marohnić) over 7 years ago Actions #6 [ruby-core:87851]

The patch got outdated in the latest trunk, so I updated it again. It's a really small change with big impact on memory usage when uploading files.

Updated by rhenium (Kazuki Yamaguchi) about 7 years ago Actions #7 [ruby-core:88355]

  • Status changed from Assigned to Closed

The patch is now in ruby/openssl (upstream)'s master branch and Ruby trunk.

For the record, the GitHub Pull Requests are:

https://github.com/ruby/ruby/pull/1924
https://github.com/ruby/openssl/pull/212

Updated by byroot (Jean Boussier) 10 months ago Actions #8

  • Related to Bug #20972: OpenSSL Memory Usage added
Actions

Also available in: PDF Atom