Bug #14920: Backport r63857 to fix performance problem on Time class in MacOs systems

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED

[rafaelfranca (Rafael França)]

Related to https://bugs.ruby-lang.org/issues/14435.

Both 2.4 and 2.5 are affected by this issue. Naruse-san commit fix it by not calling tzset() more than once when we know the year doesn't have leap seconds.

[shyouhei (Shyouhei Urabe)]

• Status changed from Open to Closed

(Changing status to closed; which triggers backporting process. This doesn't mean the request was rejected.)

[naruse (Yui NARUSE)]

• Related to Bug #14435: Time#gettime slow performance in forked process added

[naruse (Yui NARUSE)]

Maybe r63994 is better to backport.

[naruse (Yui NARUSE)]

• Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN to 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED

[rafaelfranca (Rafael França)]

Thank you for changing the status and the backport information.

Yes, that patch seems safer, but when I applied only r63994 on top of v2_4_4 tag I still see the same results running the following script.

require 'benchmark'

def create_utc_time
  100_000.times { Time.utc(2018) }
end

p "Main Process: #{Benchmark.measure { create_utc_time } }"

pid = Process.fork do
  p "Fork Process: #{Benchmark.measure { create_utc_time }}"
end

Process.wait(pid)

$ ./build/bin/ruby utc_benchmark.rb
"Main Process:   0.100000   0.000000   0.100000 (  0.101757)\n"
"Fork Process:   9.810000  27.470000  37.280000 ( 37.981360)\n"

With only r63857 I get better results.

$ ./build/bin/ruby utc_benchmark.rb
"Main Process:   0.050000   0.000000   0.050000 (  0.052233)\n"
"Fork Process:   0.050000   0.000000   0.050000 (  0.064096)\n"

[usa (Usaku NAKAMURA)]

• Backport changed from 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED to 2.3: REQUIRED, 2.4: DONE, 2.5: REQUIRED

ruby_2_4 r64560 merged revision(s) 63994.

[rafaelfranca (Rafael França)]

Hi @usa (Usaku NAKAMURA).

63994 alone doesn't fix the issue. I just cloned ruby_2_4 with r64560 and run the same benchmark script and got the same result. Only r63857 fix the issue. Can you backport r63857 too?

"Main Process:   0.100000   0.000000   0.100000 (  0.107107)\n"
"Fork Process:   9.220000   8.180000  17.400000 ( 17.605742)\n"

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.3: REQUIRED, 2.4: DONE, 2.5: REQUIRED to 2.3: REQUIRED, 2.4: DONE, 2.5: DONE

ruby_2_5 r64602 merged revision(s) 63994.

[rafaelfranca (Rafael França)]

Hi, @nagachika (Tomoyuki Chikanaga) and @usa (Usaku NAKAMURA)

Can you take another look on this backport? The commit that you backported doesn't fix the issue. I tested both ruby_2_4 and ruby_2_5 branches and the issue is still there.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.3: REQUIRED, 2.4: DONE, 2.5: DONE to 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED

Hi rafaelfranca,
Thank you for your notification.
I overlooked your comment when I backported r63994.

r63857, r63858 (and r63864 for naming convention) could affect the performance of Time.now.
I reset Backport field to express your request.
We will discuss about the side effect of these changesets.

Regards,