Changing smallvec from 1.3.0 to 1.4.1 regresses Firefox's style system performance by ~10% #243
Comments
emilio
changed the title
emilio
added a commit
to emilio/rusqlite
that referenced
this issue
Dec 4, 2020
There's no reason to use 1.4, 1.0.0 works just as well. 1.4 causes a big perf regression in Firefox, see servo/rust-smallvec#243, so while we figure that out we'd like to keep using 1.3.0.
|
I'd be curious if #241 (on master but not yet released) fixes this regression. We could publish a new release with that patch included if that makes testing easier. |
Filed #244 for this in case it's needed. |
thomcc
pushed a commit
to rusqlite/rusqlite
that referenced
this issue
Dec 5, 2020
There's no reason to use 1.4, 1.0.0 works just as well. 1.4 causes a big perf regression in Firefox, see servo/rust-smallvec#243, so while we figure that out we'd like to keep using 1.3.0.
thomcc
pushed a commit
to rusqlite/rusqlite
that referenced
this issue
Dec 5, 2020
There's no reason to use 1.4, 1.0.0 works just as well. 1.4 causes a big perf regression in Firefox, see servo/rust-smallvec#243, so while we figure that out we'd like to keep using 1.3.0.
|
I bisected this locally and I think it's the "use realloc when possible" patch what causes the regression, but will bisect in automation on Monday. Will make sure to try 1.5.1 as well, thanks @mbrubeck! |
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this issue
Dec 5, 2020
See servo/rust-smallvec#243 and the PRs in the comments. Differential Revision: https://phabricator.services.mozilla.com/D98802
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this issue
Dec 6, 2020
See servo/rust-smallvec#243 and the PRs in the comments. Do this for now since we want to uplift to fix the perf regression in 84. Differential Revision: https://phabricator.services.mozilla.com/D98802
|
Some preliminar investigation in https://bugzilla.mozilla.org/show_bug.cgi?id=1681003. It does seem related to bdfc429. I'm checking now whether this is something particular to our allocator, or a probabilistic checker we run to detect UAFs / etc, or whether it's something else. But I suspect this is a Firefox allocator issue, off-hand. |
|
Yeah, fix at https://phabricator.services.mozilla.com/D98924. |
sidvishnoi
pushed a commit
to sidvishnoi/gecko-webmonetization
that referenced
this issue
Dec 10, 2020
See servo/rust-smallvec#243 and the PRs in the comments. Differential Revision: https://phabricator.services.mozilla.com/D98802
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this issue
Dec 14, 2020
See servo/rust-smallvec#243 and the PRs in the comments. Differential Revision: https://phabricator.services.mozilla.com/D98802 UltraBlame original commit: cc2de07855f8455f16fb0bc950e1f5d6ed3774a0
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this issue
Dec 14, 2020
See servo/rust-smallvec#243 and the PRs in the comments. Differential Revision: https://phabricator.services.mozilla.com/D98802 UltraBlame original commit: cc2de07855f8455f16fb0bc950e1f5d6ed3774a0
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this issue
Dec 14, 2020
See servo/rust-smallvec#243 and the PRs in the comments. Differential Revision: https://phabricator.services.mozilla.com/D98802 UltraBlame original commit: cc2de07855f8455f16fb0bc950e1f5d6ed3774a0
dvc94ch
added a commit
to Actyx/rusqlite
that referenced
this issue
Apr 14, 2021
* Feature ptr_offset_from #41079 available in 1.47 * Fix clippy warning * Implement From/ToSql for more types This implements `FromSql` for `u64`, `usize` and `f32`, and `ToSql` for `f32`. I also updated the documentation to describe how it currently works, and changed the implementation to use `try_from` for integral casts rather rather than custom code. Test added. * Cargo format * Silence clippy * Expose query progress information * Expose query progress information Add unit tests * Rustfmt * Add ToSql implementations for u64 and usize * Reduce required lifetime Extends rusqlite#825 to - create_collation - commit_hook - rollback_hook - update_hook - table_filter * Reduce required lifetime Revert lifetime change on table_filter * Introduce Batch fallible iterator * Rustfmt * Fix Clippy warning * Reduce required lifetime in create_scalar_function * Remove lazy_static block where possible * Implement Iterator for Batch * Remove Iterator implementation for Batch If Batch implements both `Iterator` and `FaillibleIterator`, `next` method is ambiguous and must be qualified... * Get rid of fallible iterator trait * Test Batch iterator * Silence clippy's complaint about unelided lifetime in session.rs * Seal the `RowIndex` trait (technically breaking, but unlikely to break anybody) * Overhaul query API, removing the need for the `_named` variants of all functions, and `rusqlite::NO_PARAMS` * Update documentation * Deprecate NO_PARAMS in favor of passing an empty array * Ensure empty array Params impl can trigger Error::InvalidParameterCount when needed * Fix stale doc * Format code in doc comments for good measure * Add `#[inline]` and `#[cold]` in far more places * Make tests return Result * Try to fix CI build * Try to fix CI build * Fix create_collation The xDestroy callback is not called if the sqlite3_create_collation_v2() function fails. * Remove #[non_exhaustive] attribute on IndexConstraintOp * Fix non-autolinks warnings * Document that `optional()` requires import of `rusqlite::OptionalExtension` Document that `optional()` requires import of `rusqlite::OptionalExtension`. * Add/fix rustdoc links * Remove some usages of params! / NO_PARAMS * Remove obsolete doc section `FromSql` / `ToSql` are now implemented for i64 and usize * Update time to appease `deps.rs` * Upgrade to bindgen 0.56 * [docs] Fix over-long monospacing of `SQLITE_BUSY` * [docs] Document default busy_timeout/busy_handler Include a note in the documentation for both `busy_handler()` and `busy_timeout()` explaining the current default behavior (with a disclaimer indicating that this behavior should not be relied upon as it is an implementation detail that may change). * Upgrade SQLite bundled version to 3.34.0 * Use a weaker dependency on smallvec. There's no reason to use 1.4, 1.0.0 works just as well. 1.4 causes a big perf regression in Firefox, see servo/rust-smallvec#243, so while we figure that out we'd like to keep using 1.3.0. * README: document features = "window series extra_check column_decltype collation" * README: document which features imply other features * README: wrap features with quotes when needed * README: cargo test -> cargo clippy * README: caps * README: document SQLITE3_LIB/INCLUDE_DIR * README: document SQLITE3_STATIC * README: document SQLITE_MAX_* * LIBSQLITE3_FLAGS for bundled -> SQLITE_FLAGS This flag was added in a9b700c but never documented. Here we change the name to more closely follow SQLITE_MAX_VARIABLE_NUMBER etc; and document its existence in README.md. * Cargo.toml: be explicit about what bundled-full omits * revert SQLITE_FLAGS back to LIBSQLITE3_FLAGS * csvtab implies vtab * pass context to aggregate init function * add get_connection method to function context * pass context to finalize function * format * formatting and tests * format * fix lints * make init return result * Rename get_raw to get_ref_unwrap and get_raw_checked to get_ref (rusqlite#838) * test `From<FromSqlError> for Error` * Rename get_raw to get_ref_unwrap and get_raw_checked to get_ref * fix typo in .travis.yml * Fix clippy warnings * Fix CI build clippy::unnecessary_wraps is not stable yet * Use most concise syntax for params * Fix DateTime format * Fix unstable tests when machine is slow * Make it clear bind_in is not public, and inline functions passing large arrays by value to avoid too much copying * Disable leak-sanitizer for now * Remove .travis.yml * Upgrade to bindgen 0.57 * Remove travis badge * Sync series with official source * Rustfmt * Fix clippy warnings Allow `unnecessary_wraps` for `check_update` and `check_no_tail`. Remove `check_readonly` (`sqlite3-parser` may help). * Fix nightly non_fmt_panic warning * Add tests adapted from official SQLite tests * Upgrade SQLite bundled version to 3.35.0 * Upgrade SQLite bundled version to 3.35.2 * Silent some clippy warnings (rusqlite#924) * allow(clippy::upper_case_acronyms) for rust enum entries that match SQLite constants. * allow(clippy::needless_return) for collation_needed_callback until we find a way to propagate the error. * Leniently parse rfc3339 timezones (rusqlite#928) * Doctest column name reference (rusqlite#918) * Doctest column name reference * Document rusqlite assumption on column name reference And move doctest as a test. * Document when columns metadata should be extracted. * Rustfmt doc (wrap_comments) * Fix smallvec version (rusqlite#896) * Fix smallvec version https://rustsec.org/advisories/RUSTSEC-2021-0003.html * Upgrade SQLite bundled version to 3.35.4 (rusqlite#929) * Prepare release 0.25.0 (rusqlite#930) rusqlite 0.25.0 libsqlite3-sys 0.22.0 Also fix missing README for libsqlite3-sys crate And fix a typo. * Test that extra_check feature works with RETURNING statements (rusqlite#932) Test that extra_check feature works with RETURNING statements * Upgrade to bindgen 0.58 (rusqlite#933) * Use SQLITE_TEMP_STORE=3 on android. Co-authored-by: gwenn <gtreguier@gmail.com> Co-authored-by: gwenn <45554+gwenn@users.noreply.github.com> Co-authored-by: Tim Hutt <tdhutt@gmail.com> Co-authored-by: Nick Hynes <nhynes@oasislabs.com> Co-authored-by: Thom Chiovoloni <chiovolonit@gmail.com> Co-authored-by: kud1ing <kud1ing@users.noreply.github.com> Co-authored-by: Mahmoud Al-Qudsi <mqudsi@neosmart.net> Co-authored-by: Emilio Cobos Álvarez <emilio@crisal.io> Co-authored-by: Dubiousjim <dubiousjim@gmail.com> Co-authored-by: phiresky <phireskyde+git@gmail.com> Co-authored-by: Dirk Stolle <striezel-dev@web.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I suspect the realloc changes might be to blame (40fa827 / 62a3622 / bdfc429, that is, #214), or a376b02 (#228), as they're the only functional changes afaict.
The text was updated successfully, but these errors were encountered: