Skip to content

Conversation

@hauserx
Copy link
Contributor

@hauserx hauserx commented Nov 18, 2025

Pretty index was failing with following exception for files with one line and no \n at the end. The lines array was empty:

java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0

There are actually some generated files in our code based that follow this pattern.

Also, for expressions in the very first line it was creating values with negative columns, like:

x.libsonnet:2:-63

Fixed the issue and added tests, also for edge case with completely empty file.

While changing it also optimized searching for line - previously indexWhere was used which is linear search. Replaced with binary search. It does not matter to much usually, but actually was causing slowdown of profiler code that gets locations of all evaluated expressions - this will be added in a subsequent PR.

@He-Pin
Copy link
Contributor

He-Pin commented Nov 19, 2025

@hauserx Nice, seems you need run scalafmt in the shell.

@hauserx
Copy link
Contributor Author

hauserx commented Nov 19, 2025

Just noticed that the current pretty index implementation is same as in fastparse: https://github.com/com-lihaoyi/fastparse/blob/385431ec271b411505a9458bf8f85d489a9b001b/fastparse/src/fastparse/ParserInput.scala#L123

Also, sjsonnet Utils already has binary search:

def prettyIndex(lineStarts: Array[Int], index: Int): String = {

Let me change this code a little.

@hauserx
Copy link
Contributor Author

hauserx commented Nov 20, 2025

@He-Pin @stephenamar-db PTAL

@stephenamar-db stephenamar-db merged commit a571fbc into databricks:master Nov 20, 2025
6 checks passed

private lazy val lineNumberLookup: Array[Int] = {
val lines = mutable.ArrayBuffer[Int]()
val lines = mutable.ArrayBuffer[Int](0)
Copy link
Contributor

@He-Pin He-Pin Nov 20, 2025

Choose a reason for hiding this comment

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

Why change it to 0 , it will always later be resiezd then

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants