Skip to content
This repository has been archived by the owner on Aug 28, 2021. It is now read-only.

Reorganize to reduce import size of core modules. #3874

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

whiten
Copy link

@whiten whiten commented May 19, 2020

This is a huge change intended to be a proof-of-concept of reorganizing
key parts of Noms to allow its use by roci.dev/replicache-client to fit
within ~1MB of Brotli-compressed WASM. It is being shared to allow for
directional review of the scope and types of changes needed to achieve
this end, but should be broken up and reviewed as ~6 smaller commits of
more limited scope.

Major changes:

o go/d: Replace use of testify/assert with hand-rolled functions in
go/d/assert.go. Replace some previously-used functions with inline checks to
keep number of replacements down. Also replaced Equal/NotEqual() with
StringEqual/NotEqual() since this covered most cases and avoided the need for
a lot of cases in the testify code.
Saves 4,328,755 bytes (uncompressed) in repjs js/wasm.

o Separate flag registration from go/util/verbose to
go/util/verbose/verboseflags to eliminate kingpin dependency for simple
users of verbose package.

o Move http server and handlers from go/datas to go/datas/remote, adjust all
references. Some previously private elements moved to go/datas/internal so
they could be used from datas/ and datas/remote but not outside of the datas/
directory.

o Move chunks/chunk_store_common_test.go to a new chunks/chunkstest package so
it can be used outside of the chunks package.

o Move parts of go/chunks/test_utils.go (file name did not restrict this
just to tests) that use testify/assert to chunks/chunkstest, and other
parts to chunks/memory_store.go.

o Break up go/spec so that core pieces are in go/spec/lite (package name spec),
but no protocol types are registered. Users may import (as _) various
packages under go/spec (e.g., go/spec/nbs, aws, http) to install those
protocols. go/spec itself forwards most types and functions from go/spec/lite
and imports all submodules so that unmodified users of go/spec retain full
compatibility.

o Add build tags to go/types/graph_builder, opcache, opcache_compare exclude
them from the js/wasm build. These files should be moved to a new a new
package (e.g., go/types/graph), but there are various private package
members shared between these and the rest of go/types that need to be
untangled first.

Pending changes:

o go/d: Move CheckError() to new package go/d/check_error to avoid import of
kingpin, or, alternatively, require binaries using CheckError() to
register a Usage() callback to avoid the direct kingpin dependency.

o Restore functionality of RemoteDatabaseSuite currently in
go/datas/database_test.go. There's a thorny knot to unwind to allow
DatabaseSuite access to unexported members of go/datas while also
being able to use go/datas/remote without creating an import cycle.

This is a huge change intended to be a proof-of-concept of reorganizing
key parts of Noms to allow its use by roci.dev/replicache-client to fit
within ~1MB of Brotli-compressed WASM.  It is being shared to allow for
directional review of the scope and types of changes needed to achieve
this end, but should be broken up and reviewed as ~6 smaller commits of
more limited scope.

Major changes:

o go/d: Replace use of testify/assert with hand-rolled functions in
  go/d/assert.go. Replace some previously-used functions with inline checks to
  keep number of replacements down. Also replaced Equal/NotEqual() with
  StringEqual/NotEqual() since this covered most cases and avoided the need for
  a lot of cases in the testify code.
  Saves 4,328,755 bytes (uncompressed) in repjs js/wasm.

o Separate flag registration from go/util/verbose to
  go/util/verbose/verboseflags to eliminate kingpin dependency for simple
  users of verbose package.

o Move http server and handlers from go/datas to go/datas/remote, adjust all
  references. Some previously private elements moved to go/datas/internal so
  they could be used from datas/ and datas/remote but not outside of the datas/
  directory.

o Move chunks/chunk_store_common_test.go to a new chunks/chunkstest package so
  it can be used outside of the chunks package.

o Move parts of go/chunks/test_utils.go (file name did not restrict this
  just to tests) that use testify/assert to chunks/chunkstest, and other
  parts to chunks/memory_store.go.

o Break up go/spec so that core pieces are in go/spec/lite (package name spec),
  but no protocol types are registered. Users may import (as _) various
  packages under go/spec (e.g., go/spec/nbs, aws, http) to install those
  protocols. go/spec itself forwards most types and functions from go/spec/lite
  *and* imports all submodules so that unmodified users of go/spec retain full
  compatibility.

o Add build tags to go/types/graph_builder, opcache, opcache_compare exclude
  them from the js/wasm build. These files should be moved to a new a new
  package (e.g., go/types/graph), but there are various private package
  members shared between these and the rest of go/types that need to be
  untangled first.

Pending changes:

o go/d: Move CheckError() to new package go/d/check_error to avoid import of
  kingpin, or, alternatively, require binaries using CheckError() to
  register a Usage() callback to avoid the direct kingpin dependency.

o Restore functionality of RemoteDatabaseSuite currently in
  go/datas/database_test.go. There's a thorny knot to unwind to allow
  DatabaseSuite access to unexported members of go/datas while also
  being able to use go/datas/remote without creating an import cycle.
@whiten whiten marked this pull request as draft May 19, 2020 21:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant