Run benchmarks with rtoml v0.11?

samuelcolvin commented 4 days ago

I'd love to see how rtoml v0.11 (specifically https://github.com/samuelcolvin/rtoml/pull/78 where the GIL pool was removed from pyo3) effects performance.

pwwang commented 4 days ago

It's incredibly fast! See updated README.md

samuelcolvin commented 4 days ago

Nice! Any chance you could include the benchmark example from cpptoml like you used to, even if that package has been removed?

Multiple benchmark cases are useful.

pwwang commented 3 days ago

It's removed because there's a case that cannot be loaded by rtoml. I will give more details later.

pwwang commented 3 days ago

I found the culprit. It's the item here that when loaded, can't be dumped again with rtoml:

https://github.com/bobfang1992/pytomlpp/blob/47d705b909deee0fa7a3c65624bfa83ea87f578d/benchmark/data.toml#L431-L437

[terrible.capable-tent.wilderness-aromatic.birds.naughty-blind]
bake-scatter-cross = [
    234,
    { attempt-selfish-ghost = 08:43:45, draconian-ski-scintillating = 1911-03-24T12:01:06-01:20, dreary = 266 },
    'innocent broken equable adhesive spiritual title respect'
] # hard-to-find protective
ticket-enthusiastic-delightful = [ 428, 12:49:26, [ 07:10:40, 'neck unadvised finicky stale rest', true ] ]

This is the error:

[TomlSerializationError] values must be emitted before tables

A simple way to reproduce:

import rtoml

toml = """
[a]
b = [234, { c = 1, d = 2, e = 3}, 'abc']
f = [345, "def", [4, "ghi", 456]
"""

loaded = rtoml.loads(toml)
rtoml.dumps(loaded)

Error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/~/.cache/pypoetry/virtualenvs/toml-bench-TNBybMrA-py3.12/lib/python3.12/site-packages/rtoml/__init__.py", line 62, in dumps
    return serialize(obj, none_value=none_value)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_rtoml.TomlSerializationError: values must be emitted before tables

samuelcolvin commented 3 days ago

thanks, I'll look into it.

But that's not new, does that mean they've changed their examples?

pwwang commented 3 days ago

That's wired. The file was created 2 years ago.

Should I refer this to a new issue in rtoml repo? I think it fits better there.

samuelcolvin commented 3 days ago

yes please.

pwwang / toml-bench

Run benchmarks with rtoml v0.11? #7