Skip to content

Refactor def math#18847

Merged
rmuir merged 1 commit intoelastic:masterfrom
rmuir:refactor_def_math
Jun 13, 2016
Merged

Refactor def math#18847
rmuir merged 1 commit intoelastic:masterfrom
rmuir:refactor_def_math

Conversation

@rmuir
Copy link
Copy Markdown
Contributor

@rmuir rmuir commented Jun 13, 2016

This refactors operators on dynamic types to use invokedynamic (and fixes several bugs/consistency issues/tests).

The idea here is that the math operators we have now aren't that slow. Each one has a little hand written decision tree and works ok. But this gives no chance to make things better.

Instead, let the current functions be "fallback" impls, and add impls for the basic types: int, float, long, double, boolean (other types promoted). We limit the cache to a size of 1, if types change, we just revert to what we have today. The idea is to help the 90% case.

These will use all the type information we give it (which today is zero), but it gives us the opportunity to fix that, by just using a more specific indy signature. Even one argument or return value can reduce some boxing, use a simpler guard, and so on.

I fixed some consistency issues and bugs between static and dynamic cases around shifts, unary +, and bitwise operators, and added tests around this.

@jdconrad
Copy link
Copy Markdown
Contributor

LGTM!

@rmuir rmuir merged commit f8738c8 into elastic:master Jun 13, 2016
@jdconrad jdconrad mentioned this pull request Jun 14, 2016
18 tasks
map.put("lsh", PRIV_LOOKUP.findStatic(clazz, "lsh", shift));
map.put("rsh", PRIV_LOOKUP.findStatic(clazz, "rsh", shift));
map.put("ush", PRIV_LOOKUP.findStatic(clazz, "ush", shift));
return map;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this map should also be unmodifiable like the outer one. It is not so important, because the whole thing is private, but for consistency. Alternatively remove the unmodifiable from outer map, too.

@clintongormley clintongormley changed the title painless: refactor def math Refactor def math Jun 14, 2016
@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement v5.0.0-alpha4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants