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

Decoder: disallow modification of existing table #704

Merged
merged 9 commits into from Dec 15, 2021
Merged

Decoder: disallow modification of existing table #704

merged 9 commits into from Dec 15, 2021

Conversation

pelletier
Copy link
Owner

@pelletier pelletier commented Dec 15, 2021

Fixes #703. The idea is to mark tables created by key/values as explicit when entering the following table so that they can only be redefined within the same "block", not after.

This also makes two changes to the way used to track previously seen TOML objects to make up for the performance hit created by the extra marking.

First, introduce a dedicated tracker for inline tables. Because inline tables are self contained and cannot redefine anything outside of their scope, having their own tracker allows for very fast clean up (just forget about the object). To avoid allocating memory for every single inline table, a sync.Pool is used to salvage previous allocations.

Second, and more importantly: the tracking structure is now an array-backed intrusive linked list. Nodes are fully addressed using their index in the array (as opposed to the mix of ID and index). Every node points to its first child and sibling (if any). This allows operations to just consider relevant nodes, as opposed to always do range scans on the array. To make index manipulation easier (remove a lot of if idx >= 0), the structure now always contains at least one element: the root table. It is special because its siblings are used as a free list of entries. This allows reusing existing memory (less allocations), and avoids the former operation of literally moving data around to keep the array compact -- or to maintain a separate structure just for the free list.

The benchmark for SimpleDocument (literally A = "hello") is slower, as the initial setup has more overhead. It is still quite fast, so probably no need to optimize that case further.


name                               old time/op    new time/op    delta
UnmarshalDataset/config-8            18.9ms ± 0%    18.7ms ± 1%   -1.33%  (p=0.008 n=5+5)
UnmarshalDataset/canada-8            62.9ms ± 1%    62.9ms ± 1%     ~     (p=1.000 n=5+5)
UnmarshalDataset/citm_catalog-8      19.2ms ± 0%    19.1ms ± 1%     ~     (p=0.548 n=5+5)
UnmarshalDataset/twitter-8           8.84ms ± 0%    8.77ms ± 0%   -0.69%  (p=0.008 n=5+5)
UnmarshalDataset/code-8              84.2ms ± 1%    82.6ms ± 0%   -1.83%  (p=0.008 n=5+5)
UnmarshalDataset/example-8            161µs ± 1%     158µs ± 0%   -1.90%  (p=0.008 n=5+5)
Unmarshal/SimpleDocument/struct-8     433ns ± 0%     475ns ± 0%   +9.71%  (p=0.016 n=4+5)
Unmarshal/SimpleDocument/map-8        610ns ± 0%     657ns ± 1%   +7.69%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/struct-8     44.7µs ± 1%    43.7µs ± 0%   -2.26%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/map-8        63.9µs ± 1%    62.8µs ± 1%   -1.72%  (p=0.008 n=5+5)
Unmarshal/HugoFrontMatter-8          10.5µs ± 1%    10.4µs ± 0%     ~     (p=0.421 n=5+5)

name                               old speed      new speed      delta
UnmarshalDataset/config-8          55.4MB/s ± 0%  56.2MB/s ± 1%   +1.35%  (p=0.008 n=5+5)
UnmarshalDataset/canada-8          35.0MB/s ± 1%  35.0MB/s ± 1%     ~     (p=1.000 n=5+5)
UnmarshalDataset/citm_catalog-8    29.1MB/s ± 0%  29.2MB/s ± 1%     ~     (p=0.548 n=5+5)
UnmarshalDataset/twitter-8         50.0MB/s ± 0%  50.4MB/s ± 0%   +0.69%  (p=0.008 n=5+5)
UnmarshalDataset/code-8            31.9MB/s ± 1%  32.5MB/s ± 0%   +1.87%  (p=0.008 n=5+5)
UnmarshalDataset/example-8         50.2MB/s ± 1%  51.2MB/s ± 0%   +1.92%  (p=0.008 n=5+5)
Unmarshal/SimpleDocument/struct-8  25.4MB/s ± 0%  23.1MB/s ± 0%   -8.84%  (p=0.016 n=4+5)
Unmarshal/SimpleDocument/map-8     18.0MB/s ± 0%  16.7MB/s ± 1%   -7.17%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/struct-8    117MB/s ± 1%   120MB/s ± 0%   +2.31%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/map-8      82.0MB/s ± 1%  83.4MB/s ± 1%   +1.75%  (p=0.008 n=5+5)
Unmarshal/HugoFrontMatter-8        52.1MB/s ± 1%  52.3MB/s ± 0%     ~     (p=0.421 n=5+5)

name                               old alloc/op   new alloc/op   delta
UnmarshalDataset/config-8            5.92MB ± 0%    5.92MB ± 0%     ~     (p=0.056 n=5+5)
UnmarshalDataset/canada-8            84.4MB ± 0%    84.4MB ± 0%   +0.00%  (p=0.008 n=5+5)
UnmarshalDataset/citm_catalog-8      35.8MB ± 0%    35.8MB ± 0%   +0.01%  (p=0.008 n=5+5)
UnmarshalDataset/twitter-8           13.5MB ± 0%    13.5MB ± 0%   +0.20%  (p=0.008 n=5+5)
UnmarshalDataset/code-8              22.3MB ± 0%    22.3MB ± 0%   +0.00%  (p=0.008 n=5+5)
UnmarshalDataset/example-8            199kB ± 0%     194kB ± 0%   -2.22%  (p=0.008 n=5+5)
Unmarshal/SimpleDocument/struct-8      629B ± 0%      805B ± 0%  +27.98%  (p=0.008 n=5+5)
Unmarshal/SimpleDocument/map-8         957B ± 0%     1133B ± 0%  +18.39%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/struct-8     20.8kB ± 0%    20.9kB ± 0%   +0.42%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/map-8        38.1kB ± 0%    38.2kB ± 0%   +0.25%  (p=0.008 n=5+5)
Unmarshal/HugoFrontMatter-8          7.38kB ± 0%    7.46kB ± 0%   +1.08%  (p=0.008 n=5+5)

name                               old allocs/op  new allocs/op  delta
UnmarshalDataset/config-8              233k ± 0%      233k ± 0%     ~     (p=1.000 n=5+5)
UnmarshalDataset/canada-8              782k ± 0%      782k ± 0%   +0.00%  (p=0.008 n=5+5)
UnmarshalDataset/citm_catalog-8        192k ± 0%      192k ± 0%   +0.01%  (p=0.008 n=5+5)
UnmarshalDataset/twitter-8            56.9k ± 0%     56.9k ± 0%   +0.08%  (p=0.008 n=5+5)
UnmarshalDataset/code-8               1.06M ± 0%     1.06M ± 0%     ~     (all equal)
UnmarshalDataset/example-8            1.36k ± 0%     1.36k ± 0%   -0.15%  (p=0.008 n=5+5)
Unmarshal/SimpleDocument/struct-8      8.00 ± 0%      9.00 ± 0%  +12.50%  (p=0.008 n=5+5)
Unmarshal/SimpleDocument/map-8         12.0 ± 0%      13.0 ± 0%   +8.33%  (p=0.008 n=5+5)
Unmarshal/ReferenceFile/struct-8        183 ± 0%       183 ± 0%     ~     (all equal)
Unmarshal/ReferenceFile/map-8           649 ± 0%       649 ± 0%     ~     (all equal)
Unmarshal/HugoFrontMatter-8             143 ± 0%       143 ± 0%     ~     (all equal)

@pelletier pelletier added the bug Issues describing a bug in go-toml. label Dec 15, 2021
@pelletier pelletier merged commit 696dd25 into v2 Dec 15, 2021
@pelletier pelletier deleted the issue-703-2 branch December 15, 2021 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant