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

Performance regressions due to Specio #85

Closed
versable opened this issue Oct 25, 2018 · 4 comments
Closed

Performance regressions due to Specio #85

versable opened this issue Oct 25, 2018 · 4 comments

Comments

@versable
Copy link

versable commented Oct 25, 2018

Hey, first and foremost, thanks for DateTime. I have worked on custom date/time libraries in the past and I really appreciate not going through that experience again, not my cup of tea, way too complicated and too many edge cases/pitfalls.

Saying that, we are updating our infrastructure and ran into performance regressions. We noticed our application was running a bit slower on the updated platform.

After debugging with Devel::NYTProf [1] and inspecting the stack call flamegraph [2][3], we noticed that loading DateTime took 77.79% (from consuming 1.89% of our application to 8.47%) more time than previously. After investigating it was clear that this regression was introduced with commit 3308e9e.

[1] https://metacpan.org/pod/Devel::NYTProf
[2] http://www.brendangregg.com/flamegraphs.html
[3] https://github.com/brendangregg/FlameGraph

Screencaps before the regression:
dt-params

dt-params-time

Screencaps after the regression:
dt-specio

dt-specio-time

dt-specia-validate-time

@autarch
Copy link
Member

autarch commented Oct 25, 2018

Thanks for the report.

Just to clarify. This output is purely about the load time of the module, not its runtime performance, right?

The fact that it loads more slowly is something I already knew. However, I felt that this was worth it given that Specio led to a 14% improvement in object construction speed. See https://blog.urth.org/2016/06/05/making-datetime-faster-and-slower/ for my own benchmarking from back when I first worked on switching to Specio.

If you have any thoughts on how to speed up the load time, I'd be all for it. Looking at the profile, it seems like this mostly comes down to two things. The first is that it's loading many more modules. Most of those load very quickly.

The second is that there's a lot of time spent in Specio::OO. This isn't surprising, since that particular module is used at load time by most of the other modules being loaded. It calls eval many times to create constructors and attribute accessor methods for the caller. Off hand, I don't see any obvious way to speed this up, but I haven't really looked into it much.

@autarch
Copy link
Member

autarch commented Oct 26, 2018

Also, can you share whatever code you profiled? I'm not sure exactly how to reproduce that locally. Note that I know how to use NYTProf, I'm just asking what you profiled.

autarch added a commit to houseabsolute/Specio that referenced this issue Oct 26, 2018
@autarch
Copy link
Member

autarch commented Oct 26, 2018

So I investigated this a bit and I can optimize Specio's load time a small amount, but I don't think it would ever be possible to make DateTime as fast to load as it used to be. That said, as I noted in my blog post, I think it's much more useful to optimize DateTime's runtime performance, and using Params::ValidationCompiler and Specio made quite an improvement there.

autarch added a commit to houseabsolute/Specio that referenced this issue Oct 26, 2018
autarch added a commit to houseabsolute/Specio that referenced this issue Oct 26, 2018
clrpackages pushed a commit to clearlinux-pkgs/perl-Specio that referenced this issue Oct 29, 2018
…n 0.43

0.43     2018-10-26

- Optimized compile-time operations to make Specio itself quicker to
  load. Specio's load time is a non-trivial part of the load time of DateTime
  (and presumably other things that use it). Based on
  houseabsolute/DateTime.pm#85. Reported by
  versable.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Nov 25, 2018
Upstream changes:
0.43     2018-10-26

- Optimized compile-time operations to make Specio itself quicker to
  load. Specio's load time is a non-trivial part of the load time of DateTime
  (and presumably other things that use it). Based on
  houseabsolute/DateTime.pm#85. Reported by
  versable.
@autarch
Copy link
Member

autarch commented Jan 4, 2019

I'm going to close this since it's not really actionable at this point. PRs to speed up load time for DateTime, Specio, or any other deps I work on would be welcome.

@autarch autarch closed this as completed Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants