In MVEL .empty can be way way way slower then .isEmpty() #5086

Closed
nik9000 opened this Issue Feb 11, 2014 · 5 comments

Comments

Projects
None yet
3 participants
@nik9000
Contributor

nik9000 commented Feb 11, 2014

This happens when you use .empty to check if a field is empty and it actually is empty. I've made a gist to reproduce it: https://gist.github.com/nik9000/8937202

High level:

  1. Hit ~1 million documents with a script score.
  2. Do something like (doc['foo'].empty ? 0 : doc['foo'].value) * doc['bar']. The .empty is the key here.
  3. If most of the documents don't have a foo then this is really slow. Like, two seconds slow.
  4. Instead, switch to this (doc['foo'].isEmpty() ? 0 : doc['foo'].value) * doc['bar']. That is faster. .36 seconds or so. Not super speedy, but much better.

I'm not completely sure what is happening but my guess is that the MVEL optimizer is optimizing .empty is a call to .isEmpty on an instance of either ScriptDocValues or ScriptDocValues.Empty. When it hits the wrong one it recovers by catching an IllegalArgumentException thrown by the JVM and then does some munging. The act of filling in the stack trace for that IllegalArgumentException is really really really slow. So slow, I see it in the hot threads: https://gist.github.com/nik9000/8937335 .

Workaround: use .isEmpty() instead of .empty.

I'm not sure what the fix ought to be.

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Feb 14, 2014

Contributor

No one has responded in a few days and I don't have time to do much other than the work around. Would it be reasonable for me to add this information to the documentation until someone has time to fix this?

Contributor

nik9000 commented Feb 14, 2014

No one has responded in a few days and I don't have time to do much other than the work around. Would it be reasonable for me to add this information to the documentation until someone has time to fix this?

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Feb 14, 2014

Contributor

@nik9000 sorry you opened that while 1.0 was so hot I couldn't touch anything else - gimme another day or two :)

Contributor

s1monw commented Feb 14, 2014

@nik9000 sorry you opened that while 1.0 was so hot I couldn't touch anything else - gimme another day or two :)

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Feb 14, 2014

Contributor

Cool cool.

Contributor

nik9000 commented Feb 14, 2014

Cool cool.

@clintongormley

This comment has been minimized.

Show comment
Hide comment
@clintongormley

clintongormley Dec 29, 2014

Member

MVEL has been removed. Closing

Member

clintongormley commented Dec 29, 2014

MVEL has been removed. Closing

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Dec 29, 2014

Contributor

Thanks! Groovy has much better performance and seems much more stable!
On Dec 29, 2014 6:37 AM, "Clinton Gormley" notifications@github.com wrote:

Closed #5086 #5086.


Reply to this email directly or view it on GitHub
#5086 (comment)
.

Contributor

nik9000 commented Dec 29, 2014

Thanks! Groovy has much better performance and seems much more stable!
On Dec 29, 2014 6:37 AM, "Clinton Gormley" notifications@github.com wrote:

Closed #5086 #5086.


Reply to this email directly or view it on GitHub
#5086 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment