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

builtin_flatten_arrays: expected array, got string #156

Closed
ben-manes opened this issue Apr 5, 2024 · 9 comments
Closed

builtin_flatten_arrays: expected array, got string #156

ben-manes opened this issue Apr 5, 2024 · 9 comments

Comments

@ben-manes
Copy link

ben-manes commented Apr 5, 2024

I tried evaluating a complex jsonnet file using the latest from jrsonnet-unstable-builds:
jrsonnet 0.0.0-git.7f29d2da65f7623c71566a288af9988797d9691d

The error seems to dislike the std.flattenArrays usage in the code snippet.

The evaluation works fine in go-jsonnet. I was just doing a quick inspection for our team using go-jsonnet, where their evaluation times are taking 1-3 minutes and was curious if this implementation might assist them. Not sure if I can offer more insights, so you are welcome to close this issue if not detailed enough for you.

code
getIconForPlatform(iconName)::
  // Please add to this list as needed; this is not an exhaustive list of icons and you will need to define platform equivalents manually.
  // See app/Components/AtomicElements/Atoms/Icon/Icon.tsx for the list of available mobile icons and
  // node_modules/@blueprintjs/datetime/node_modules/@blueprintjs/core/dist/generated/iconClasses.js for the list of available web/mobile web icons
  local icons = [
    { mobile: 'done', web: 'pt-icon-tick-circle', mobileWeb: self.web, alternateNames: ['checkmark', 'check'] },
    { mobile: 'clock', web: 'pt-icon-time', mobileWeb: self.web },
    { mobile: 'edit', web: 'pt-icon-edit', mobileWeb: self.web },
    { mobile: 'map-pin', web: 'pt-icon-pin', mobileWeb: self.web },
    { mobile: 'chat', web: 'pt-icon-chat', mobileWeb: self.web },
    { mobile: 'chevron-right', web: 'pt-icon-chevron-right', mobileWeb: self.web },
    { mobile: 'arrow-left', web: 'pt-icon-arrow-left', mobileWeb: self.web },
  ];
  local getIcon = std.prune([
    if std.member(std.flattenArrays(std.objectValues(icon)), iconName) then icon[platform]
    for icon in icons
  ]);
  if std.length(getIcon) > 0 then getIcon[0] else iconName,
error
type error: expected array, got string
    argument <arrs> evaluation
    ../../library/uiHelpers/components/components.libsonnet:730:28-70:                            function <builtin_flatten_arrays> call
    argument <arr> evaluation
    ../../library/uiHelpers/components/components.libsonnet:730:17-81:                            function <builtin_member> call
    ../../library/uiHelpers/components/components.libsonnet:730:5-730:6:                          if condition
    <std>:223:53-55:                                                                              variable <x> access
    <std>:222:20-22:                                                                              variable <a> access
    argument <v> evaluation
    <std>:222:8-23:                                                                               function <builtin_is_array> call
    <std>:222:8-222:9:                                                                            if condition
    <std>:223:45-56:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:223:35-57:                                                                              function <isContent> call
    ../../library/uiHelpers/components/components.libsonnet:729:4-729:5:                          function <prune> call
    ../../library/uiHelpers/components/components.libsonnet:733:26-34:                            variable <getIcon> access
    argument <x> evaluation
    ../../library/uiHelpers/components/components.libsonnet:733:15-35:                            function <builtin_length> call
    ../../library/uiHelpers/components/components.libsonnet:733:3-733:4:                          if condition
    ../../library/uiHelpers/components/components.libsonnet:736:4-736:5:                          function <getIconForPlatform> call
    ../../library/uiHelpers/components/components.libsonnet:740:4-740:5:                          variable <icon> access
    <std>:227:32-34:                                                                              field <name> access
    <std>:222:20-22:                                                                              variable <a> access
    argument <v> evaluation
    <std>:222:8-23:                                                                               function <builtin_is_array> call
    <std>:222:8-222:9:                                                                            if condition
    <std>:227:20-36:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:227:10-37:                                                                              function <isContent> call
    <std>:223:45-56:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:223:35-57:                                                                              function <isContent> call
    <std>:227:20-36:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:227:10-37:                                                                              function <isContent> call
    <std>:223:45-56:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:223:35-57:                                                                              function <isContent> call
    ../../library/basebowl/helpers/basebowlUtils.libsonnet:463:19-463:20:                         function <prune> call
    <std>:227:32-34:                                                                              field <children> access
    <std>:222:20-22:                                                                              variable <a> access
    argument <v> evaluation
    <std>:222:8-23:                                                                               function <builtin_is_array> call
    <std>:222:8-222:9:                                                                            if condition
    <std>:227:20-36:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:227:10-37:                                                                              function <isContent> call
    <std>:223:45-56:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:223:35-57:                                                                              function <isContent> call
    <std>:227:20-36:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:227:10-37:                                                                              function <isContent> call
    <std>:223:45-56:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:223:35-57:                                                                              function <isContent> call
    <std>:227:20-36:                                                                              function <prune> call
    <std>:214:10-12:                                                                              variable <b> access
    <std>:214:13-214:14:                                                                          if condition
    <std>:227:10-37:                                                                              function <isContent> call
    ../../library/utils.libsonnet:59:5-59:6:                                                      function <prune> call
    ../../library/renderer/renderer.libsonnet:29:21-139:                                          function <pick> call
    ../../library/renderer/renderer.libsonnet:44:47-57:                                           variable <currProps> access
    ../../library/renderer/renderer.libsonnet:47:46-60:                                           variable <combinedProps> access
    ../../library/renderer/renderer.libsonnet:7:29-35:                                            variable <props> access
    ../../library/component/base.libsonnet:126:36-42:                                             variable <props> access
    ../../library/component/base.libsonnet:90:60-66:                                              variable <props> access
    ../../library/component/base.libsonnet:126:15-151:                                            function <normalize> call
    <std>:222:20-22:                                                                              variable <a> access
    argument <v> evaluation
    <std>:222:8-23:                                                                               function <builtin_is_array> call
    <std>:222:8-222:9:                                                                            if condition
    ../../library/component/base.libsonnet:126:5-152:                                             function <prune> call
    ../../library/renderer/renderer.libsonnet:7:3-68:                                             function <newFromJson> call
    ../../library/renderer/renderer.libsonnet:47:27-93:                                           function <mapComponent> call
    ../../library/renderer/renderer.libsonnet:76:5-21:                                            variable <mappedComponent> access
    <std>:222:20-22:                                                                              variable <a> access
    argument <v> evaluation
    <std>:222:8-23:                                                                               function <builtin_is_array> call
    <std>:222:8-222:9:                                                                            if condition
    ../../library/renderer/renderer.libsonnet:75:4-75:5:                                          function <prune> call
    ../../library/renderer/renderer.libsonnet:85:40-114:                                          function <render> call
    ../../library/basebowl/stories/driver/scenes/preArrival/preArrival.scene.libsonnet:388:19-55: function <mobile> call
    field <uiSchema> manifestification
    field <mobile> manifestification
    field <uiView> manifestification
    field <scenes> manifestification
    field <core_storyboard_plan> manifestification
@CertainLach
Copy link
Owner

CertainLach commented Apr 6, 2024

In jrsonnet, std.flattenArrays only accepts array of arrays, while in go-jsonnet it works with strings too.

However, the way it works with strings in go-jsonnet isn't good nor intuitive:

std.flattenArrays(['a', 'b', 'c', ['d', 'e']]) == "[ ]abc[\"d\", \"e\"]"

I don't think that's what you want here anyway?

@CertainLach
Copy link
Owner

CertainLach commented Apr 6, 2024

I see there is flattenDeepArray in jsonnet standard library now, which work as expected:

std.flattenDeepArray(['a', 'b', 'c', ['d', 'e']]) == ['a', 'b', 'c', 'd', 'e']`)

But isn't implemented in jrsonnet/go-jsonnet yet:
https://github.com/google/jsonnet/blob/master/stdlib/std.jsonnet#L883-L887

I will implement it in jrsonnet, but in meanwhile, you can just use it in your code

local flattenDeepArray(value) = 
  if std.isArray(value) then
    [y for x in value for y in flattenDeepArray(x)]
  else
    [value];

flattenDeepArray(['a', 'b', 'c', ['d', 'e']]) == ['a', 'b', 'c', 'd', 'e']

@CertainLach CertainLach added this to the v0.5.0 milestone Apr 6, 2024
@ben-manes
Copy link
Author

ben-manes commented Apr 7, 2024

Thanks! Perhaps it would make sense to have a compat mode and emit warnings to help teams transition?

I made the change and it worked! I had to set the max-stack size, but it dropped the runtime from 145s to 12s. I know they run this in VSCode. Is there an ability to swap that to use yours?

time jsonnet deployment/heartland.plan.jsonnet -o /tmp/plan-go.json
jsonnet deployment/heartland.plan.jsonnet -o /tmp/plan-go.json  144.82s user 1.11s system 223% cpu 1:05.37 total

time ~/Downloads/jrsonnet deployment/heartland.plan.jsonnet -o /tmp/plan-rust.json --max-stack 400
~/Downloads/jrsonnet deployment/heartland.plan.jsonnet -o /tmp/plan-rust.json  12.19s user 0.58s system 98% cpu 12.985 total

diff -q /tmp/plan-go.json /tmp/plan-rust.json && echo "identical" || echo "different"
identical

@CertainLach
Copy link
Owner

Thanks! Perhaps it would make sense to have a compat mode and emit warnings to help teams transition?

Maybe I can add reasons why functions are typed this way to error messages, but it isn't possible to implement the compat mode in general.

In this case, accepting strings isn't even the intended behavior, it is just upstream jsonnet lacking type checks.

I made the change and it worked! I had to set the max-stack size, but it dropped the runtime from 145s to 12s. I know they run this in VSCode. Is there an ability to swap that to use yours?

You mean, how to use jrsonnet for file preview in VSCode? As I can see, grafana language server uses embedded go-jsonnet for evaluation, and it is not possible right now to use jrsonnet instead. It should be possible to implement this feature, however.

CertainLach added a commit that referenced this issue Apr 7, 2024
@ben-manes
Copy link
Author

That's all very reasonable. I think having improved warnings is always nice. For VSCode, we already use the Save and Run extension for scripts so writing a shortcut that invokes jrsonnet should be easy.

One last question while I'm bothering you, do you have any advice on how to profile / optimize jsonnet? Presumably our 2.5 minute evaluations are abnormal and perhaps trivial to fix if we understood what the root cause was. We tried profiling go-jsonnet (pprof svg) which showed flattenArrays as a bottleneck, but at the interpreter level rather than our own code.

@CertainLach
Copy link
Owner

That is complicated.

Some years ago I was trying to optimize my jsonnet deployments, and everything was bottlenecked at the interpreter. I did some domain-specific optimizations, but every time I came to conclusion that original implementations are optimized for conditions, that are wastly different from my code (And from code of kube-prometheus and other projects), so instead I just decided to write my own jsonnet implementation, and there it is :D

After couple of years, some optimizations were properly implemented in upstream, but jrsonnet/sjsonnet are still just much better.

@CertainLach
Copy link
Owner

CertainLach commented Apr 7, 2024

In latest release I have also included faster implementation of std.prune, so some things might just start working faster for you (I see some time was spent in it on your stack traces).

Standard implementation of prune is evaluating everything exponentially more times, it is first calling itself for every element of array/object (recursively), and then doing the same thing to provide result.

As none of jsonnet implementations implement memoization, this is one of examples that will work poorly (In jrsonnet, prune is now much more optimized.)

  prune(a)::
    local isContent(b) =
      if b == null then
        false
      else if std.isArray(b) then
        std.length(b) > 0
      else if std.isObject(b) then
        std.length(b) > 0
      else
        true;
    if std.isArray(a) then
      [std.prune(x) for x in a if isContent($.prune(x))]
    else if std.isObject(a) then {
      [x]: $.prune(a[x])
      for x in std.objectFields(a)
      if isContent(std.prune(a[x]))
    } else
      a,

And for the demo why standard prune is bad...

local
  prune(a)=std.trace('prune call!',
    local isContent(b) = 
      if b == null then
        false
      else if std.isArray(b) then
        std.length(b) > 0
      else if std.isObject(b) then
        std.length(b) > 0
      else
        true;
    if std.isArray(a) then
      [prune(x) for x in a if isContent(prune(x))]
    else if std.isObject(a) then {
      [x]: prune(a[x])
      for x in std.objectFields(a)
      if isContent(prune(a[x]))
    } else
      a),
  tower(depth) = if depth == 0 then 'el' else local v = tower(depth - 1); [v, v, v];

prune(tower(5))

prune call! will be logged 2005 times here, and it will be much more for larger object trees.

@ben-manes
Copy link
Author

ben-manes commented Apr 7, 2024

oh wow, that really knocked the time down! I built master locally and now it runs in less than a second 😮

time ~/Downloads/jrsonnet-r deployment/heartland.plan.jsonnet -o /tmp/plan-rust.json
~/Downloads/jrsonnet-r deployment/heartland.plan.jsonnet -o   0.35s user 0.05s system 96% cpu 0.410 total

We use jsonnet for developing client-side workflows that we deploy to shipment facilities to streamline the truck turnaround by having the driver, guard, clerk, forklift operator all collaborate on their devices (video). So these create pretty large, customized workflow plans that we deploy as json assemblies, and our solution engineers wrote a framework in jsonnet to standardize our components. That was a pretty huge productivity boost, but then slow build times killed that again so this is pretty amazing.

@ben-manes
Copy link
Author

ben-manes commented Apr 8, 2024

Shared with the impacted team this morning, lots of happy people thanks to your work 😁

Wow, just tested it out. That is incredible. The plan took 500+ seconds to evaluate before and now 1.41s user 0.15s system 98% cpu 1.575 total

This is life changing.
Before: 418.64s user 3.89s system 220% cpu 3:11.63 total
After: 1.45s user 0.13s system 95% cpu 1.645 total

and the founder on Slack after I added it to the Sunday night company update,

can we talk about the JSONNET speedup, that’s huge
It takes our SE’s 3.5 minutes to just see if their change(s) work, our engineering got it down to 0.5 seconds
that’s such a huge improvement

I gave them this snippet to integrate into VS Code until we put something nicer together,

"saveAndRun": {
  "commands": [
    {
      "match": "\\.(jsonnet|libsonnet)$",
      "cmd": "jrsonnet ${file} &> /tmp/results.json ; code --reuse-window /tmp/results.json",
      "useShortcut": true,
      "silent": true
    }
  ]
}

anyway, just wanted you know this project is greatly appreciated!

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

No branches or pull requests

2 participants