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 issue with invokedynamic + Rack + URI #2493

Closed
mje113 opened this issue Jan 21, 2015 · 5 comments
Closed

Performance issue with invokedynamic + Rack + URI #2493

mje113 opened this issue Jan 21, 2015 · 5 comments

Comments

@mje113
Copy link

@mje113 mje113 commented Jan 21, 2015

While benchmarking a simple Rack app with and without invokedynamic, I noticed that calls to URI::Generic.default_port were taking an excessive amount of time with invokedynamic turned on.

Note 19 seconds spent calling default_port with indy on, vs nothing with it off:
https://gist.github.com/mje113/988f065369dd5693dffb

@mje113
Copy link
Author

@mje113 mje113 commented Jan 21, 2015

Oops--forgot to mention that the method in question:
https://github.com/jruby/jruby/blob/master/lib/ruby/stdlib/uri/generic.rb#L30

If changed to return DEFAULT_PORT instead of self::DEFAULT_PORT seems to fix the performance issue.

@headius
Copy link
Member

@headius headius commented Jan 22, 2015

The perf improvement from removing self:: means this is likely a problem with how the indy logic compiles the :: form of constant lookup (called "Const2" in our AST). Will investigate.

@headius
Copy link
Member

@headius headius commented Jan 22, 2015

Simple answer...there's NO caching at all being done for this form (inheritance_search_const in IR), so the constant and all the invokedynamic method handles to wrap it are getting created every time.

@headius
Copy link
Member

@headius headius commented Jan 22, 2015

Hmm actually it looks like we are attempting to cache it until the global constant invalidator fires or the target module is not the same one as last time. Adding some logging, it looks like that invalidation is not working right, or else we're not able to cache this variable properly:

2015-01-21T19:05:15.872-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::Generic>
2015-01-21T19:05:15.873-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::HTTP>
2015-01-21T19:05:15.874-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::Generic>
2015-01-21T19:05:15.875-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::HTTP>
2015-01-21T19:05:15.876-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::Generic>
2015-01-21T19:05:15.877-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::HTTP>
2015-01-21T19:05:15.877-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::Generic>
2015-01-21T19:05:15.878-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::HTTP>
2015-01-21T19:05:15.879-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::Generic>
2015-01-21T19:05:15.880-06:00: Bootstrap: DEFAULT_PORT  retrieved and cached from type #<Class:URI::HTTP>
@headius
Copy link
Member

@headius headius commented Jan 22, 2015

See #2495 for a description of the problem.

@headius headius added this to the 9.0.0.0.pre2 milestone Jan 22, 2015
@headius headius self-assigned this Jan 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants