callgrind on the tokenizer showed SWAR::has_special_char and eq_mask
compiled as calls, one per eight bytes; paged_file_impl::size() out of line
behind every eof() and remaining(); and the cursor's page-cache check not
inlined into peek() because it shared a function with the page fetch. The
SWAR helpers are forced inline, size() is defined in the class, and
cached_() is split into an inline check and an out-of-line refresh.
TXG (58 MB), single thread: tokenizer 196 -> 204 MB/s in memory and
136 -> 193 MB/s through 64 KB pages; strict parse through pages
1.15 -> 0.97 s against 0.93 s in memory; lazy index pass over pages
218 -> 311 MB/s; lazy open 0.52 -> 0.43 s.
This commit was written by an AI coding tool and has not been verified by
a human.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wcN7XquTfUi4vsKQ4KchL
Every peek(), get() and SWAR word read on a paged reader went through the
page cache's hash map and LRU list, so tokenizing through pages ran at a
fifth of the speed of the in-memory buffer. The reader now remembers the
page its cursor was last on and serves reads that fall inside it from the
pointer, revalidated against an eviction counter on the implementation
so a page that left the cache is never read through a stale pointer.
Reads that straddle a page boundary take the existing paths.
The whole tokenizer over each file, 64 KB pages, 64 cached (4 MB):
in-memory buffer paged before paged after
TXG 58 MB 200 MB/s 42 MB/s 142 MB/s
210_King 148 MB 181 MB/s 34 MB/s 118 MB/s
OKgate22 232 MB 197 MB/s 38 MB/s 128 MB/s
This is the step that makes reading in pages a candidate for the default
path rather than a fallback; the remaining gap is the page fetch itself.
This commit was written by an AI coding tool and has not been verified by
a human.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wcN7XquTfUi4vsKQ4KchL
Each instance was four separate allocations: the instance_data record, the
attribute array object it pointed at, that array's index bytes, and its
slot storage. A 58 MB model made 10.4 million mallocs to load 918k
instances, and massif attributed 99 MB of its 450 MB peak to malloc
bookkeeping alone.
variant_array now allocates the size byte, the per-slot type indices and
the slots as one block, and instance_data holds the array in a
std::optional instead of behind a pointer (an empty optional keeps the
meaning the null pointer had: attribute storage constructed on the fly
from the RocksDB backend). No ownership or lifetime changes; the same
object owns the same data.
Parse, C++ file constructor, on top of the previous commits:
TXG 58 MB 0.96 -> 0.94 s steady 310 -> 262 MB peak 383 -> 335 MB
210_King 148 MB 2.74 -> 2.63 s steady 758 -> 630 MB peak 948 -> 820 MB
OKgate22 232 MB 3.70 -> 3.61 s steady 1165 -> 971 MB peak 1447 -> 1252 MB
mallocs while loading TXG: 10.39M -> 7.56M.
This commit was written by an AI coding tool and has not been verified by
a human.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wcN7XquTfUi4vsKQ4KchL
byguid_ was a std::map<std::string, ...>: a red-black node plus a
heap-allocated 22-character string per rooted instance, and a lookup that
walks ~18 levels of string comparisons on a 200k-entry file.
guid_map keeps keys of up to 23 characters inline in an unordered_map node
(every valid GlobalId is 22), and routes anything longer to an ordered map
so invalid files still work. Same std::string-keyed interface as before.
Parse, C++ file constructor, on top of the previous commits:
TXG 58 MB 1.08 s -> 1.03 s 341 -> 335 MB
210_King 148 MB 2.80 s -> 2.72 s 836 -> 830 MB
OKgate22 232 MB 4.25 s -> 3.94 s 1271 -> 1253 MB
This commit was written by an AI coding tool and has not been verified by
a human.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wcN7XquTfUi4vsKQ4KchL
full_buffer_impl::size(), get(), get_u32() and get_u64() (and their mmap_impl
twins) were defined out of line, so every character the lexer read crossed a
call boundary with its own bounds check. Callgrind put the three at 5.6% of
parse self time; inlining them lets the compiler hoist the checks out of the
scanning loops, which is worth more than their own cost.
Also reserve the streamer's inverse vector from the file size (about one
record per 32 bytes of SPF on real models) and shrink it once the bulk load
is sorted, so the doubling copies and the capacity slack go away.
Parse time, C++ file constructor, 12-core Linux box:
TXG 58 MB 1.35 s -> 1.15 s
210_King 148 MB 3.70 s -> 3.09 s
OKgate22 232 MB 6.18 s -> 5.30 s
Python ifcopenshell.open(): 1.40 -> 1.22, 3.70 -> 3.24, 6.27 -> 5.52 s.
Memory unchanged. The removed exported symbols mean the Python wrapper must
be rebuilt against this library.
This commit was written by an AI coding tool and has not been verified by
a human.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wcN7XquTfUi4vsKQ4KchL
Also drop-in replacement mostly, except extra cmake args need to be provided now after `--` - `python run-cmake.py vs2022-x64 -- -DGLTF_SUPPORT=ON`.
Internally, script relies on env variables much much less.
Mainly drop-in replacement for `build-ifcopenshell.bat` with `--help`, kw args and args validation, but there's a small caveat.
Previously it was possible to pass args to the underlying build tool as simple positionals - e.g. `build-ifcopenshell vs2022-x64 Release /p:Foo=bar`.
This behaviour is disabled now, because it doesn't allow validating provided args - it's impossible to tell whether `--config Release` is meant to be passed to msbuild or was meant as `--build-cfg Release` for `build-ifcopenshell`.
But it's still possible to pass args to msbuild by using `--` - `python build-ifcopenshell.py vs2022-x64 Release -- /p:Foo=bar`
`install-ifcopenshell.py` is now just a small wrapper passing `--target INSTALL` arg.
* Rewrite to_string() to use switch{} and handle Token_NONE and identifier without as_string()
* Add regression tests for to_string() on tokens without a string form
Cover both halves of the recursion that made a whitespace-only file
segfault: token::to_string() on the EOF marker and on an instance name,
and a parse of input that lexes to zero tokens, which is how the header
parser reaches token::as_string() on the EOF marker.
Generated with the assistance of an AI coding tool.
---------
Co-authored-by: Bruno Postle <bruno@postle.net>
process_deletion_inverse() called inverse_index::remove_source(), which
walked every record in the file's inverse index to find the ones whose
source is the deleted instance: O(R) per deletion, the dominant cost of
file.remove() on large files now that the lookup side no longer re-sorts.
The records a deleted instance contributed are exactly the entity
references in its own attributes, so walk those with the same visitor
build_inverses_() uses for registration and remove each record with a
targeted binary search instead. remove_source() has no callers left and
is deleted.
Also use the ordered view of batch_deletion_ids_ (a boost multi_index
that already had one) for the is-this-referencer-also-being-deleted
check in process_deletion_(), which was a linear std::find over the
sequenced view: O(b) per referencing instance made batch deletion of b
instances quadratic.
file.remove on 300 IfcPropertySet of a 155 MB IFC4 model (201k IfcRoot)
drops from 3.15 ms to 0.17 ms per call, batched removal of 2000 from
3.34 ms to 0.17 ms per call, root.remove_product on 100 walls from
332 ms to 131 ms per call.
Claude-Session: https://claude.ai/code/session_01HNrXDmR88wKPCYwGE21SyH
(cherry picked from commit 938442303f)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Requiring quotes around a decimal was a surprise with no way to predict it:
Foobar.Baz>1 works, Foobar.Baz>1.5 is a syntax error, and > >= < <= are
almost always used on numbers. Writing Width>"0.2" also reads like a text
comparison even though it is not.
The "." is excluded from unquoted_string because it separates a pset from a
property, but that separator has already been consumed by the time a value is
read, so a number is unambiguous in value position. Add a decimal_string
alternative to the value rule only. Signed and leading dot forms are accepted,
1. is not, and pset, prop and query keys are unchanged - they still need
quoting for a ".".
The transformer returns the same string a quoted value would, so =1.5 and
="1.5" are the same query and compare() does the existing type coercion.
Nothing that parses today changes meaning; only inputs that used to be a
syntax error now work.
Update the quoting docs accordingly, and assert the Bay property that the
filter test set up but never checked.
Generated with the assistance of an AI coding tool.
An unquoted pset, prop, key or value may not contain any of , . = > < * !
or whitespace. This was never stated, and the only motivation the docs gave
for quoting was the "Level 3" example having a space, so there was nothing
to suggest that Pset_WallCommon.ThermalTransmittance=1.5 is a syntax error
while FireRating=2HR is fine. Decimal numbers are the common way to hit it.
State the rule where the reader meets values, in a new subsection under each
of the two value tables. The two grammars exclude different characters, so
the element value section states its own set and only the delta. Note that
all of these characters are a syntax error except the comma, which is read
as the filter separator instead, so Name=Foo,IfcWall silently means "named
Foo and an IfcWall" rather than matching the literal value.
Also fix the Query filter example, which had the same problem in the file
itself: query:types.count=0 does not error, it is silently parsed as a
property filter for a count property in a property set named query:types,
and matches nothing. Quote the keys, as the other query examples already do.
Generated with the assistance of an AI coding tool.
Moving to Python to make Windows build scripts more maintainable.
It's intended to be a drop-in replacement, so it should be possible to just switch `.\build-deps.cmd` to `python build-deps.py`, keeping exactly the same arguments and behaviour will be the same.
`build-deps.cmd` is deprecated, but not yet removed, but will be shortly after more testing.
Other batch files will be migrated to Python shortly after too.
Apparently `ty` is being too strict here and warning about `Any` possibly being `PathLike` which is not supported on older Pythons.
```
error[deprecated]: The overload of `which` is deprecated
--> src\bonsai\bonsai\bim\module\drawing\operator.py:2267:34
|
2267 | command[0] = shutil.which(command[0]) or command[0]
| ^^^^^^^^^^^^ On Windows before Python 3.12, using a PathLike as `cmd` would always fail or return `None`.
error[deprecated]: The overload of `which` is deprecated
--> src\bonsai\bonsai\tool\drawing.py:1324:30
|
1324 | command[0] = shutil.which(command[0]) or command[0]
| ^^^^^^^^^^^^ On Windows before Python 3.12, using a PathLike as `cmd` would always fail or return `None`.
```
Commented out long time ago in ec04644.
Nowadays static linking would be done differently - using `CMAKE_MSVC_RUNTIME_LIBRARY`, but we don't really support static runtime.
Example error that occurred:
```
CMake Error in CMakeLists.txt:
IMPORTED_LOCATION not set for imported target "RocksDB::rocksdb"
configuration "MinSizeRel".
```
manifold was introduced in a19d398, but its `goto :manifold` occurred too early in the file skipping bunch of dependencies - I guess that's why it was removed in 6ad5fbb, but then new target, qt was using `Successful` as the next label, so `manifold` end up being always skipped.
Making it reachable as we do use it in nix/build-all.
CGAL never runs `find_package` for `Boost`, `GMP` or `MPFR` during its configuration and never hardcodes their paths to the generated configs. So providing them have no effect. It's also can be confirmed by `build-deps.cmd` on Windows running all this time without the most of these args. Though it was settings `BOOST_ROOT` but it had no effect too.
Probably it's some kind of artifact from CGAL past when it's used to be non-header-only library.
Caching of version path was introduced in c5f6d47, awhile ago, removing it, since all new installations are already using the cache instead of this default fallback.
`robocopy` line was an artifact from old layout.
It was only needed to set `PYTHON_INSTALLER` correctly, but
`PYTHON_INSTALLER` is actually only used when arch is not x64/arm64,
exactly the only cases when `PYTHON_AMD64_POSTFIX` is set.
Dion Moult