Closed Bug 1600906 Opened 7 months ago Closed 6 months ago

Reduce state in dom/indexedDB classes

Categories

(Core :: Storage: IndexedDB, task, P3)

task

Tracking

()

RESOLVED FIXED

People

(Reporter: sg, Assigned: sg)

References

(Regressed 1 open bug)

Details

Attachments

(31 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
No description provided.
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6c086ca5780
Make use of FlippedOnce in VersionChangeTransaction. r=dom-workers-and-storage-reviewers,ytausky
Keywords: leave-open
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/69dba2abeaf4
Make use of FlippedOnce in Database. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/fcea7ad7ad7b
Make use of FlippedOnce in FullIndexMetadata. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/09e4368e2355
Make use of FlippedOnce in FullObjectStoreMetadata. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/7c4e2c13f1c3
Make use of FlippedOnce in ConnectionPool. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/dc96d3709a42
Make use of FlippedOnce in QuotaClient. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/4e0758ff6af7
Make use of FlippedOnce in DatabaseInfo. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/4fe02c654acc
Make use of FlippedOnce in ThreadRunnable. r=dom-workers-and-storage-reviewers,ytausky
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c73a68b20dde
Make use of FlippedOnce in TransactionBase. r=dom-workers-and-storage-reviewers,ytausky
Regressions: 1602683
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/69f0ebc63a61
Extended InitializedOnce to allow restricting the valid values. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/0989faf6b5f1
Reduce statefulness of TransactionBase using const and InitializedOnce. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/7a3e7318e28e
Made FlippedOnce member functions constexpr. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/26d96c797b0f
Reduce statefulness of DatabaseConnection using InitializedOnce. r=dom-workers-and-storage-reviewers,ytausky
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9b4c307ed8a4
Use move semantics with ThreadInfo. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/bcee5e565d96
Use move semantics with IdleDatabaseInfo. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/cd458b52a7ea
Encapsulate DatabaseOperationBase::mResultCode. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/e722062cbfc2
Reduce statefulness of TransactionDatabaseOperationBase using InitializedOnce. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/1ed684598bd0
Disable remote settings blocklist when running indexedDB tests, as this heavily uses IndexedDB. r=dom-workers-and-storage-reviewers,ytausky
Regressions: 1603767

This also simplifies delegating calls that are dependent on the cursor type.

Also reduce dependency on IDBCursor.h by moving enums and type traits to IDBCursorType.h

Depends on D57992

Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/498d5e653e97
Avoid unnecessary copy. r=dom-workers-and-storage-reviewers,ttung
https://hg.mozilla.org/integration/autoland/rev/5b13eeec7485
Merge identical switch cases. r=dom-workers-and-storage-reviewers,ttung
Attachment #9117251 - Attachment description: Bug 1600906 - Use std::move instead of swap, use const where then possible. r=#dom-workers-and-storage → Bug 1600906 - Encapsulate ThreadInfo; use std::move instead of swap, use const where then possible. r=#dom-workers-and-storage
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8b95dac9290
Encapsulate ThreadInfo; use std::move instead of swap, use const where then possible. r=dom-workers-and-storage-reviewers,ytausky
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8f7410826381
Use std::move instead of swap, use const where then possible. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/801f97f67366
Use InitializedOnce in DirectoryInfo to allow moving instances without additional state. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/94ca5d86b3b0
Use scoped enums in IDBCursor. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/4d7b95550368
Avoid copying key buffers. r=dom-workers-and-storage-reviewers,ytausky

This change reduces the number of data members and state in individual classes
and ensures that for each cursor type only the data members actually used
exist at compile time rather than requiring various assertions and case
distinctions at run-time. The mapping from the type known at run-time to the
compile-time types is done at a single code location in
TransactionBase::AllocCursor.

The ObjectStore vs. Index and Key vs. Value cursor type aspects are now properly
separated in different classes.

This patch also renames the misnamed CursorTypeTraits in ActorsParent.cpp to
PopulateResponseHelper, which is necessary as part of this patch to resolve
the ambiguity with the CursorTypeTraits defined in IDBCursorType.h

It also eliminates the CursorPosition type, which is redundant with the
CursorData template defined in IDBCursorType.h, which is now reused on both
the parent and child sides to avoid unnecessary heterogeneity.

Depends on D57993

Formerly, a mixture of uint64_t, int64_t and even int32_t was used. Now,
int64_t is used via an IndexOrObjectStoreId type alias. int64_t is the
type used in PBackgroundIDBSharedTypes.ipdlh

Depends on D59478

Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a114b9b8e6ea
Convert IDBCursor and BackgroundCursorChild to templates to increase type safety and reduce state. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/d61a65e2e4ac
Transform Cursor into a template depending on the cursor type. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/aae2faf1907c
Harmonize types for index and object store ids. r=dom-workers-and-storage-reviewers,ytausky

Backed out for bustages on StaticAnalysisFunctions.h

backout: https://hg.mozilla.org/integration/autoland/rev/da9250ac90ba0ebed64a9141e73e0b8cec52daed

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-fuzzing%2Fdebug%2C%28bf%29&revision=aae2faf1907c41bad94d8b71b3fcf1f0e1bf9a55

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284407081&repo=autoland&lineNumber=73798

[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/nsError.h:15,
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/nscore.h:203,
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - from /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.h:10,
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - from /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:7:
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp: In member function 'bool mozilla::dom::indexedDB::{anonymous}::Cursor<CursorType>::VerifyRequestParams(const mozilla::dom::indexedDB::CursorRequestParams&, mozilla::dom::indexedDB::{anonymous}::CursorPosition<CursorType>&) const':
[task 2020-01-10T14:49:44.990Z] 14:49:44 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/StaticAnalysisFunctions.h:65:47: error: no match for 'operator!' (operand type is 'mozilla::dom::indexedDB::{anonymous}::CursorBase')
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - # define MOZ_CHECK_ASSERT_ASSIGNMENT(expr) (!!(expr))
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - ^~~~~~~
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/Likely.h:17:48: note: in definition of macro 'MOZ_UNLIKELY'
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - # define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - ^

Flags: needinfo?(sgiesecke)

(In reply to Natalia Csoregi [:nataliaCs] from comment #50)

Backed out for bustages on StaticAnalysisFunctions.h

... on the Linux x64 debug build-linux64-fuzzing/debug and Linux x64 debug hazard-linux64-haz/debug (H) builds only. Regular builds seem to be fine. I am reaching out to the fuzzing team to better understand this.

Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e65d7e9dea10
Convert IDBCursor and BackgroundCursorChild to templates to increase type safety and reduce state. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/970b5ea41af0
Transform Cursor into a template depending on the cursor type. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/2aa2a5d78d34
Harmonize types for index and object store ids. r=dom-workers-and-storage-reviewers,ytausky
Flags: needinfo?(sgiesecke)

I consider this bug done by now. State has been significantly reduced. Future similar work should be done under a new bug.

Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.