[core/state] Do not initialize a sparse array with a long List[str]

[akinomyoga]

Fixes #2263.

This fixes an issue of #2263. This also fixes another issue #2263 (comment). In the previous versions, unset[-1]=1 has created unset=(1) in the Python version and has core-dumped in the C++ version.

$ bash -c 'unset[-1]=1; declare -p unset'
bash: line 1: unset[-1]: bad array subscript
$ bin/osh -c 'unset[-1]=1; declare -p unset'
declare -a unset=(1)
$ osh-0.27.0 -c 'unset[-1]=1; declare -p unset'  # 0.27.0 C++ version
Segmentation fault         (core dumped) osh-0.27.0 -c 'unset[-1]=1; declare -p unset'

The last commit 069162f adds a spec test for the above bug.

This PR contains another commit a271b53 to refactor the creation of a new sparse array. We've been passed an empty List[str] to BashArray_FromList(list), but it is unnecessary to generate any list for an empty new sparse array. This commit adds a new function BashArray_New().

[akinomyoga]

	Python version	C++ version	Bash 5.2	Bash 5.3-beta
Without eval_unsafe_arith	69.30 MB (9.17s)	38.23 MB (0.313s)	31.97 MB (0.364s)	14.77 MB (0.178s)
With eval_unsafe_arith	69.29 MB (9.28s)	38.57 MB (0.311s)	N/A	N/A

Now, both the speed and the memory size are comparable to Bash. The C++ version is faster than Bash 5.2 and twice as slow as Bash 5.3. The memory size is similar to Bash 5.2 and about 2.5x as Bash 5.3.

[andychu]

Oh awesome!

Do we know why bash 5.3 got faster, and uses less memory? Is it because of some bash malloc changes?

[andychu]

Excellent, thank you!!

Good to see the internal APIs making it easy to handle the error cases

[akinomyoga]

Do we know why bash 5.3 got faster, and uses less memory? Is it because of some bash malloc changes?

There has been a ridiculous bug in Bash 4.3..5.2, where two copies of a function were generated for every function (and one was not used at all). Actually, I submitted a patch to Bash last June:

https://lists.gnu.org/archive/html/bug-bash/2024-06/msg00000.html

[andychu]

Wow, that's funny! Great find

(OSH does have benchmarks in the CI, so we would find anything that doubles memory usage that way)

---

Oh well now I am a bit disappointed we're slower than bash 5.3 :-/ There is a GC-optimized build at _bin/cxx-opt+souffle/osh, but I tried it, and it's not much faster (less GC rooting)

YSH can be a lot faster, but I guess Python/mycpp/OSH does a lot of pointer chasing compared to C code, even though there is less copying

(I wonder if "small string optimization" will help ble.sh in particular, since it uses many integer-like strings? That would save strings of 6 bytes inside the pointer, rather than allocating a GC object)

[andychu]

(Also it's weird that bash doesn't have a good commit history - bminor/bash@ba4ab055 )

[akinomyoga]

Oh well now I am a bit disappointed we're slower than bash 5.3 :-/ There is a GC-optimized build at _bin/cxx-opt+souffle/osh, but I tried it, and it's not much faster (less GC rooting)

Yeah, but I think what's measured by source ble.sh is just the function generation speed. I believe the execution speed is a different story.

(I wonder if "small string optimization" will help ble.sh in particular, since it uses many integer-like strings? That would save strings of 6 bytes inside the pointer, rather than allocating a GC object)

I believe it should help in general (though I'm not sure if it helps particularly ble.sh). Does the current master branch already implement it? Or is it a future plan?

[andychu]

Yeah, but I think what's measured by source ble.sh is just the function generation speed. I believe the execution speed is a different story.

Yeah that's true, for source of a file with many functions, then the interpreter is doing one thing

1. parsing the function bodies
2. entering it in a dict

And we are ~1x-3x slower on parsing (the 3x is the result of bash 5.2 optimizations I think)

https://oils.pub/release/0.27.0/benchmarks.wwz/osh-parser/

---

So yeah I agree the runtime speed is more important, since we are already at a "reasonable" speed (faster than bash 5.2)

Small string optimization isn't turned on, but I prototyped it here - https://github.com/oils-for-unix/oils/blob/master/mycpp/small_str_test.cc#L35

Although in general I think bigger speed ups will come from the work that @melvinw has done

• GC rooting optimization - this has a big speedup on the "bubble sort" microbenchmark, although I tested _bin/cxx-opt+souffle/osh on the SparseArray benchmarks, and it was only a small difference
• some kind of value types optimization -- not sure if/when we will get here, but chasing many pointers is probably expensive
  • this interacts with a few other things, so it's not straightforward - I have a blog post draft that mentions some of them

But it's good that we've fixed this BashArray problem by moving to the sparse representation. I'm glad it actually worked as intended! :-) :-) :-)