dirnodes could cache encrypted/serialized entries for speed #329
Labels
No labels
c/code
c/code-dirnodes
c/code-encoding
c/code-frontend
c/code-frontend-cli
c/code-frontend-ftp-sftp
c/code-frontend-magic-folder
c/code-frontend-web
c/code-mutable
c/code-network
c/code-nodeadmin
c/code-peerselection
c/code-storage
c/contrib
c/dev-infrastructure
c/docs
c/operational
c/packaging
c/unknown
c/website
kw:2pc
kw:410
kw:9p
kw:ActivePerl
kw:AttributeError
kw:DataUnavailable
kw:DeadReferenceError
kw:DoS
kw:FileZilla
kw:GetLastError
kw:IFinishableConsumer
kw:K
kw:LeastAuthority
kw:Makefile
kw:RIStorageServer
kw:StringIO
kw:UncoordinatedWriteError
kw:about
kw:access
kw:access-control
kw:accessibility
kw:accounting
kw:accounting-crawler
kw:add-only
kw:aes
kw:aesthetics
kw:alias
kw:aliases
kw:aliens
kw:allmydata
kw:amazon
kw:ambient
kw:annotations
kw:anonymity
kw:anonymous
kw:anti-censorship
kw:api_auth_token
kw:appearance
kw:appname
kw:apport
kw:archive
kw:archlinux
kw:argparse
kw:arm
kw:assertion
kw:attachment
kw:auth
kw:authentication
kw:automation
kw:avahi
kw:availability
kw:aws
kw:azure
kw:backend
kw:backoff
kw:backup
kw:backupdb
kw:backward-compatibility
kw:bandwidth
kw:basedir
kw:bayes
kw:bbfreeze
kw:beta
kw:binaries
kw:binutils
kw:bitcoin
kw:bitrot
kw:blacklist
kw:blocker
kw:blocks-cloud-deployment
kw:blocks-cloud-merge
kw:blocks-magic-folder-merge
kw:blocks-merge
kw:blocks-raic
kw:blocks-release
kw:blog
kw:bom
kw:bonjour
kw:branch
kw:branding
kw:breadcrumbs
kw:brians-opinion-needed
kw:browser
kw:bsd
kw:build
kw:build-helpers
kw:buildbot
kw:builders
kw:buildslave
kw:buildslaves
kw:cache
kw:cap
kw:capleak
kw:captcha
kw:cast
kw:centos
kw:cffi
kw:chacha
kw:charset
kw:check
kw:checker
kw:chroot
kw:ci
kw:clean
kw:cleanup
kw:cli
kw:cloud
kw:cloud-backend
kw:cmdline
kw:code
kw:code-checks
kw:coding-standards
kw:coding-tools
kw:coding_tools
kw:collection
kw:compatibility
kw:completion
kw:compression
kw:confidentiality
kw:config
kw:configuration
kw:configuration.txt
kw:conflict
kw:connection
kw:connectivity
kw:consistency
kw:content
kw:control
kw:control.furl
kw:convergence
kw:coordination
kw:copyright
kw:corruption
kw:cors
kw:cost
kw:coverage
kw:coveralls
kw:coveralls.io
kw:cpu-watcher
kw:cpyext
kw:crash
kw:crawler
kw:crawlers
kw:create-container
kw:cruft
kw:crypto
kw:cryptography
kw:cryptography-lib
kw:cryptopp
kw:csp
kw:curl
kw:cutoff-date
kw:cycle
kw:cygwin
kw:d3
kw:daemon
kw:darcs
kw:darcsver
kw:database
kw:dataloss
kw:db
kw:dead-code
kw:deb
kw:debian
kw:debug
kw:deep-check
kw:defaults
kw:deferred
kw:delete
kw:deletion
kw:denial-of-service
kw:dependency
kw:deployment
kw:deprecation
kw:desert-island
kw:desert-island-build
kw:design
kw:design-review-needed
kw:detection
kw:dev-infrastructure
kw:devpay
kw:directory
kw:directory-page
kw:dirnode
kw:dirnodes
kw:disconnect
kw:discovery
kw:disk
kw:disk-backend
kw:distribute
kw:distutils
kw:dns
kw:do_http
kw:doc-needed
kw:docker
kw:docs
kw:docs-needed
kw:dokan
kw:dos
kw:download
kw:downloader
kw:dragonfly
kw:drop-upload
kw:duplicity
kw:dusty
kw:earth-dragon
kw:easy
kw:ec2
kw:ecdsa
kw:ed25519
kw:egg-needed
kw:eggs
kw:eliot
kw:email
kw:empty
kw:encoding
kw:endpoint
kw:enterprise
kw:enum34
kw:environment
kw:erasure
kw:erasure-coding
kw:error
kw:escaping
kw:etag
kw:etch
kw:evangelism
kw:eventual
kw:example
kw:excess-authority
kw:exec
kw:exocet
kw:expiration
kw:extensibility
kw:extension
kw:failure
kw:fedora
kw:ffp
kw:fhs
kw:figleaf
kw:file
kw:file-descriptor
kw:filename
kw:filesystem
kw:fileutil
kw:fips
kw:firewall
kw:first
kw:floatingpoint
kw:flog
kw:foolscap
kw:forward-compatibility
kw:forward-secrecy
kw:forwarding
kw:free
kw:freebsd
kw:frontend
kw:fsevents
kw:ftp
kw:ftpd
kw:full
kw:furl
kw:fuse
kw:garbage
kw:garbage-collection
kw:gateway
kw:gatherer
kw:gc
kw:gcc
kw:gentoo
kw:get
kw:git
kw:git-annex
kw:github
kw:glacier
kw:globalcaps
kw:glossary
kw:google-cloud-storage
kw:google-drive-backend
kw:gossip
kw:governance
kw:grid
kw:grid-manager
kw:gridid
kw:gridsync
kw:grsec
kw:gsoc
kw:gvfs
kw:hackfest
kw:hacktahoe
kw:hang
kw:hardlink
kw:heartbleed
kw:heisenbug
kw:help
kw:helper
kw:hint
kw:hooks
kw:how
kw:how-to
kw:howto
kw:hp
kw:hp-cloud
kw:html
kw:http
kw:https
kw:i18n
kw:i2p
kw:i2p-collab
kw:illustration
kw:image
kw:immutable
kw:impressions
kw:incentives
kw:incident
kw:init
kw:inlineCallbacks
kw:inotify
kw:install
kw:installer
kw:integration
kw:integration-test
kw:integrity
kw:interactive
kw:interface
kw:interfaces
kw:interoperability
kw:interstellar-exploration
kw:introducer
kw:introduction
kw:iphone
kw:ipkg
kw:iputil
kw:ipv6
kw:irc
kw:jail
kw:javascript
kw:joke
kw:jquery
kw:json
kw:jsui
kw:junk
kw:key-value-store
kw:kfreebsd
kw:known-issue
kw:konqueror
kw:kpreid
kw:kvm
kw:l10n
kw:lae
kw:large
kw:latency
kw:leak
kw:leasedb
kw:leases
kw:libgmp
kw:license
kw:licenss
kw:linecount
kw:link
kw:linux
kw:lit
kw:localhost
kw:location
kw:locking
kw:logging
kw:logo
kw:loopback
kw:lucid
kw:mac
kw:macintosh
kw:magic-folder
kw:manhole
kw:manifest
kw:manual-test-needed
kw:map
kw:mapupdate
kw:max_space
kw:mdmf
kw:memcheck
kw:memory
kw:memory-leak
kw:mesh
kw:metadata
kw:meter
kw:migration
kw:mime
kw:mingw
kw:minimal
kw:misc
kw:miscapture
kw:mlp
kw:mock
kw:more-info-needed
kw:mountain-lion
kw:move
kw:multi-users
kw:multiple
kw:multiuser-gateway
kw:munin
kw:music
kw:mutability
kw:mutable
kw:mystery
kw:names
kw:naming
kw:nas
kw:navigation
kw:needs-review
kw:needs-spawn
kw:netbsd
kw:network
kw:nevow
kw:new-user
kw:newcaps
kw:news
kw:news-done
kw:news-needed
kw:newsletter
kw:newurls
kw:nfc
kw:nginx
kw:nixos
kw:no-clobber
kw:node
kw:node-url
kw:notification
kw:notifyOnDisconnect
kw:nsa310
kw:nsa320
kw:nsa325
kw:numpy
kw:objects
kw:old
kw:openbsd
kw:openitp-packaging
kw:openssl
kw:openstack
kw:opensuse
kw:operation-helpers
kw:operational
kw:operations
kw:ophandle
kw:ophandles
kw:ops
kw:optimization
kw:optional
kw:options
kw:organization
kw:os
kw:os.abort
kw:ostrom
kw:osx
kw:osxfuse
kw:otf-magic-folder-objective1
kw:otf-magic-folder-objective2
kw:otf-magic-folder-objective3
kw:otf-magic-folder-objective4
kw:otf-magic-folder-objective5
kw:otf-magic-folder-objective6
kw:p2p
kw:packaging
kw:partial
kw:password
kw:path
kw:paths
kw:pause
kw:peer-selection
kw:performance
kw:permalink
kw:permissions
kw:persistence
kw:phone
kw:pickle
kw:pip
kw:pipermail
kw:pkg_resources
kw:placement
kw:planning
kw:policy
kw:port
kw:portability
kw:portal
kw:posthook
kw:pratchett
kw:preformance
kw:preservation
kw:privacy
kw:process
kw:profile
kw:profiling
kw:progress
kw:proxy
kw:publish
kw:pyOpenSSL
kw:pyasn1
kw:pycparser
kw:pycrypto
kw:pycrypto-lib
kw:pycryptopp
kw:pyfilesystem
kw:pyflakes
kw:pylint
kw:pypi
kw:pypy
kw:pysqlite
kw:python
kw:python3
kw:pythonpath
kw:pyutil
kw:pywin32
kw:quickstart
kw:quiet
kw:quotas
kw:quoting
kw:raic
kw:rainhill
kw:random
kw:random-access
kw:range
kw:raspberry-pi
kw:reactor
kw:readonly
kw:rebalancing
kw:recovery
kw:recursive
kw:redhat
kw:redirect
kw:redressing
kw:refactor
kw:referer
kw:referrer
kw:regression
kw:rekey
kw:relay
kw:release
kw:release-blocker
kw:reliability
kw:relnotes
kw:remote
kw:removable
kw:removable-disk
kw:rename
kw:renew
kw:repair
kw:replace
kw:report
kw:repository
kw:research
kw:reserved_space
kw:response-needed
kw:response-time
kw:restore
kw:retrieve
kw:retry
kw:review
kw:review-needed
kw:reviewed
kw:revocation
kw:roadmap
kw:rollback
kw:rpm
kw:rsa
kw:rss
kw:rst
kw:rsync
kw:rusty
kw:s3
kw:s3-backend
kw:s3-frontend
kw:s4
kw:same-origin
kw:sandbox
kw:scalability
kw:scaling
kw:scheduling
kw:schema
kw:scheme
kw:scp
kw:scripts
kw:sdist
kw:sdmf
kw:security
kw:self-contained
kw:server
kw:servermap
kw:servers-of-happiness
kw:service
kw:setup
kw:setup.py
kw:setup_requires
kw:setuptools
kw:setuptools_darcs
kw:sftp
kw:shared
kw:shareset
kw:shell
kw:signals
kw:simultaneous
kw:six
kw:size
kw:slackware
kw:slashes
kw:smb
kw:sneakernet
kw:snowleopard
kw:socket
kw:solaris
kw:space
kw:space-efficiency
kw:spam
kw:spec
kw:speed
kw:sqlite
kw:ssh
kw:ssh-keygen
kw:sshfs
kw:ssl
kw:stability
kw:standards
kw:start
kw:startup
kw:static
kw:static-analysis
kw:statistics
kw:stats
kw:stats_gatherer
kw:status
kw:stdeb
kw:storage
kw:streaming
kw:strports
kw:style
kw:stylesheet
kw:subprocess
kw:sumo
kw:survey
kw:svg
kw:symlink
kw:synchronous
kw:tac
kw:tahoe-*
kw:tahoe-add-alias
kw:tahoe-admin
kw:tahoe-archive
kw:tahoe-backup
kw:tahoe-check
kw:tahoe-cp
kw:tahoe-create-alias
kw:tahoe-create-introducer
kw:tahoe-debug
kw:tahoe-deep-check
kw:tahoe-deepcheck
kw:tahoe-lafs-trac-stream
kw:tahoe-list-aliases
kw:tahoe-ls
kw:tahoe-magic-folder
kw:tahoe-manifest
kw:tahoe-mkdir
kw:tahoe-mount
kw:tahoe-mv
kw:tahoe-put
kw:tahoe-restart
kw:tahoe-rm
kw:tahoe-run
kw:tahoe-start
kw:tahoe-stats
kw:tahoe-unlink
kw:tahoe-webopen
kw:tahoe.css
kw:tahoe_files
kw:tahoewapi
kw:tarball
kw:tarballs
kw:tempfile
kw:templates
kw:terminology
kw:test
kw:test-and-set
kw:test-from-egg
kw:test-needed
kw:testgrid
kw:testing
kw:tests
kw:throttling
kw:ticket999-s3-backend
kw:tiddly
kw:time
kw:timeout
kw:timing
kw:to
kw:to-be-closed-on-2011-08-01
kw:tor
kw:tor-protocol
kw:torsocks
kw:tox
kw:trac
kw:transparency
kw:travis
kw:travis-ci
kw:trial
kw:trickle
kw:trivial
kw:truckee
kw:tub
kw:tub.location
kw:twine
kw:twistd
kw:twistd.log
kw:twisted
kw:twisted-14
kw:twisted-trial
kw:twitter
kw:twn
kw:txaws
kw:type
kw:typeerror
kw:ubuntu
kw:ucwe
kw:ueb
kw:ui
kw:unclean
kw:uncoordinated-writes
kw:undeletable
kw:unfinished-business
kw:unhandled-error
kw:unhappy
kw:unicode
kw:unit
kw:unix
kw:unlink
kw:update
kw:upgrade
kw:upload
kw:upload-helper
kw:uri
kw:url
kw:usability
kw:use-case
kw:utf-8
kw:util
kw:uwsgi
kw:ux
kw:validation
kw:variables
kw:vdrive
kw:verify
kw:verlib
kw:version
kw:versioning
kw:versions
kw:video
kw:virtualbox
kw:virtualenv
kw:vista
kw:visualization
kw:visualizer
kw:vm
kw:volunteergrid2
kw:volunteers
kw:vpn
kw:wapi
kw:warners-opinion-needed
kw:warning
kw:weapi
kw:web
kw:web.port
kw:webapi
kw:webdav
kw:webdrive
kw:webport
kw:websec
kw:website
kw:websocket
kw:welcome
kw:welcome-page
kw:welcomepage
kw:wiki
kw:win32
kw:win64
kw:windows
kw:windows-related
kw:winscp
kw:workaround
kw:world-domination
kw:wrapper
kw:write-enabler
kw:wui
kw:x86
kw:x86-64
kw:xhtml
kw:xml
kw:xss
kw:zbase32
kw:zetuptoolz
kw:zfec
kw:zookos-opinion-needed
kw:zope
kw:zope.interface
p/blocker
p/critical
p/major
p/minor
p/normal
p/supercritical
p/trivial
r/cannot reproduce
r/duplicate
r/fixed
r/invalid
r/somebody else's problem
r/was already fixed
r/wontfix
r/worksforme
t/defect
t/enhancement
t/task
v/0.2.0
v/0.3.0
v/0.4.0
v/0.5.0
v/0.5.1
v/0.6.0
v/0.6.1
v/0.7.0
v/0.8.0
v/0.9.0
v/1.0.0
v/1.1.0
v/1.10.0
v/1.10.1
v/1.10.2
v/1.10a2
v/1.11.0
v/1.12.0
v/1.12.1
v/1.13.0
v/1.14.0
v/1.15.0
v/1.15.1
v/1.2.0
v/1.3.0
v/1.4.1
v/1.5.0
v/1.6.0
v/1.6.1
v/1.7.0
v/1.7.1
v/1.7β
v/1.8.0
v/1.8.1
v/1.8.2
v/1.8.3
v/1.8β
v/1.9.0
v/1.9.0-s3branch
v/1.9.0a1
v/1.9.0a2
v/1.9.0b1
v/1.9.1
v/1.9.2
v/1.9.2a1
v/cloud-branch
v/unknown
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: tahoe-lafs/trac#329
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
We found that for large directories (353 entries), it takes a non-trivial
amount of time to serialize the child entries into a single string: 500ms.
This currently represents about 5% of the time necessary to update the
directory, and might grow to 10% once we make some other performance
improvements.
I suspect (but do not have confirmation) that most of this time is spent
encrypting the child write-caps. We do a couple of hashes, an HMAC, and a
short AES encryption for each one.
Since most updates only modify one row, we could consider caching the
serialized forms and re-using them during update. Basically _unpack_contents
would return a dict mapping childname to (name, rocap, rwcap, metadata,
serialized), and all update functions would put a None in the 'serialized'
slot. _pack_contents would use the 'serialized' slot if present, otherwise it
would re-serialize the other parameters.
This is only a minor improvement, so I expect it will be a long time before
we decide to implement it, but I wanted to get the ideas down before we
forgot.
See also #414 (profiling on directory unpacking) and #383 (large directories take a long time to modify).
See also #327 (performance measurement of directories).
Kevan Carstensen expressed interest in working on this.
So, summarizing discussions with zooko and warner...
We first need to handle testing for backwards compatibility. This means that we'll want to capture a base32-encoded representation of the output of _pack_contents as it sits now when run against some small, known directory structure. We'll then use that to test _unpack_contents (to make sure that the unpacked directory structure is what we're expecting), and _pack contents (to make sure that packing the same known directory structure results in the same packed representation).
Then we can do the optimizations that this ticket suggests.
(that didn't really need to be a list, huh.)
I've started work on the first point by writing a little program to try to get the base32 representation of the output of _pack_contents. Unfortunately, I run into path issues when I try to run it.
At the top of my script, I have
to tell the Python interpreter about tahoe's support directory. The goal was to have a somewhat less robust version of the detection that the tahoe executable does when it first starts (since I know where the directory is on my system, and don't imagine that the script is going to be run elsewhere, since it's kind of a one-time deal to make tests work).
When I run the script, and try to import something from the tests directory, I get
which would seem to suggest that my path hacking doesn't do what I want it to.
Is there something easy that I'm missing that would make this work?
(also, my approach for getting this information is based on what of tahoe's code I've read, which isn't much. if there's a more obvious way to get the strings that we need, or if my program is flawed in some other way, please let me know)
Attachment get_dirnode_strings.py (1322 bytes) added
a first shot at making a test program
The way to do that is to set the
PYTHONPATH
environment variable before invoking thepython
interpreter. There are some hacks (mostly due to setuptools, I think) that need to run at interpreter startup time and learn about the available Python packages, so adding to yoursys.path
after interpreter startup won't work.It was a good idea though.
If you can't or don't want to set your
PYTHONPATH
, then you'll need to find thefoolscap
package, probably in/path-to-tahoe/tahoe/support/lib/python2.6/site-packages/foolscap-0.4.2-py2.6.egg
, and add that whole path (including thething.egg
part) to yoursys.path
, and then do likewise with the other packages.That fixed the issue -- thanks.
Just so I'm clear on the next part:
Currently,
_unpack_contents
returns a dictionary mapping the name of each child node to a pair(child node instance, metadata)
, where metadata is a dictionary of metadata._pack_contents
gets therocap
andrwcap
(if present) from the child node instance. What we want to do is_unpack_contents
to return a dict mapping the name of each child node to a triple(child node instance, metadata, serialized)
, where child node instance and metadata are as they are now, and serialized is the serialized from of the other data._pack_contents
to deal with this new representation; specifically, to useserialized
if present instead of serializing the other data, which we think is expensive.Adder
,MetadataSetter
andDeleter
to work with these changes; specifically, they should, when modifying a node, set the correspondingserialized
value to None, which will tell_pack_contents
that it needs to re-serialize that node._pack_contents
to the new_unpack_contents
, we should get the same underlying directory structure back (i.e.: all expected child node names are present, and metadata values are as they should be. Is there anything else we'd want to check?)_pack_contents
should be the same as the output from the old one, given the same directory structure.Adder
,Deleter
, andMetadataSetter
in the same way that we do now. Are there already tests to adequately verify this, or should I plan on writing those, too?(apologies if this is pedantic, but I'm still getting the hang of tahoe, and don't want to go off on an irrelevant tangent because I've made a bad assumption somewhere)
That sounds good to me!
I usually write code like this by adding a new unit test. You can run a
specific unit test with a minimum of fuss (and startup time and extraneous
noise) by doing e.g.
make quicktest TEST=allmydata.test.test_dirnode.DeepStats.test_stats
.That will get all the PYTHONPATH stuff set up for you. Anything you
print
from the unit test will get displayed, so I use this for one-offtools all the time.
Also, you can use
misc/run-with-pythonpath.py
, which will set up theright environment and then run the command of your choice. For example, if
you wrote a
foo.py
to do the stuff you just described, then you couldinvoke
python misc/run-with-pythonpath.py python foo.py
.I'm disappointed that the sys.path technique you tried didn't work.. it used
to.
Your approach to
_pack_contents
sounds great! Note that theserialized
form should contain the wholename+rocap+encrwcap+metadata
string (i.e. be sure to include the name).That way, if
_pack_contents
sees that it has a pre-serialized stringavailable, it can just append that to the list of entries that it's building,
and doesn't need to re-serialize the childname either. At the end of that
loop, it should do a single
"".join(entries)
to perform the finalassembly (rather than doing incremental
+=
operations, which would be alot slower).
We should make sure that the rocap/rwcap is the same too. Doing
child.get_uri() and comparing it against a constant is plenty.
test_dirnode.py should already have test coverage for the modifiers. It
exercises all the
NewDirectoryNode
methods, likeset_uri
andlist
. If they pass, then you've updated the modifier classessuccessfully.
Also, if it works on your platform (it's somewhat touchy right now), use our
test-coverage tools:
make quicktest-figleaf figleaf-output
, and checkto see that all the code you've touched is actually being run. The buildbot
has a link to the current coverage data (generated under py2.4; it seems that
py2.5 gives slightly different answers). I think it should be pretty easy to
make these changes and achieve the same or better coverage than before.
Also, I don't know if it'd be worth it, but you could get fancy and instead
of changing the data structure from a list of two-tuples to a list of
three-tuples, you could:
list
*setitem*
to remove that auxilliary valueget_both_items
method, which takes a key and returns a two-tuple of (aux-serialized, value) (which is really (aux-serialized, (child,metadata)))set_both_items
method which takes (key, aux-serialized, (child,metadata))Adder
andDeleter
don't have to change: they'll set the entries that have changed and leave the rest alone_pack_contents
usesget_both_items
and prefers the pre-serialized form if it's still thereSince
NewDirectoryNode.list
returns whatever_unpack_contents
returns, you might manage to write less code if you use this technique.
Otherwise you'll need to find all the callers of
_read
and update themto tolerate the new three-tuple (and add a wrapper to
list
to convertit into two-tuple form for external callers, so that it continues to honor
the same interface defined in
allmydata.interfaces.IDirectoryNode
).Oh, and of course, it would be a good idea to actually measure the time that serialization/deserialization takes before doing any of this work. Create 10/100/1k/10k/100k entries at random, call
_pack_contents
undertimeit.py
or some other sort of how-long-does-it-take loop, and figure out anA+Bx
curve (I'd expect the serialization time to be some constant A plus some per-entry time B, let's figure out what A and B are). Let's make sure that serialization is the culprit, it might be that call to_create_node
in unpack that we should focus on. And let's set a reasonable goal.. maybe we should be able to unpack+modify+pack 10k entries in less than a second, or something.The measuring part is my job -- #327 (performance measurement of directories).
Okay, I've got tests in place -- I'll attach them in a bit.
One minor nit: I was working on the assumption that
_pack_contents
and_unpack_contents
were inverses -- i.e., if I do_pack_contents(_unpack_contents(string))
, I should getstring
back. I made this a condition in my test function, but it failed --_pack_contents
would return a different string, even when fed with the unmodified dict returned from_unpack_contents
. But all of the data inside the dict seem the same -- at least the rocaps, rwcaps, and metadata. Is this normal?Also, the hardcoding of the base32-encoded representation of that directory tree is kind of horrific -- any ideas for a better way of doing that?
source:src/allmydata/dirnode.py#L201
The reason is that the encryption of the write-cap uses a random IV. There isn't an easy way to make that deterministic and still secure right now, so we can't rely on it for testing.
(Hm, for future reference -- i.e. after Tahoe v1.5.0 -- maybe we could generate the random IV as the secure hash of the write cap itself. That would be make it deterministic without, as far as I can currently see, losing security.)
As to the base32-encoded directory tree, that looks okay to me.
Okay. Given that, do you think that the other tests in
test_unpack_and_pack_behavior()
are going to be enough?I've finished doing a few things.
_pack_contents
and_unpack_contents
intest_dirnode.py
to not check the output of_pack_contents
for equality, since that will always fail at the moment. I've also moved a misplaced comment.CachingDict
, the data structure described by warner above (cool idea, by the way).CachingDict
todirnode.py
. It basically does what warner suggested that it do._pack_contents
,_unpack_contents
, and_create
to useCachingDict
s instead ofdict
s.With these done (tests.txt, dict.txt, optimizations.txt applied),
python setup.py test
passes on my machine.Attachment tests.txt (26728 bytes) added
tests for _unpack_contents, _pack_contents, and CachingDict
Attachment optimizations.txt (22867 bytes) added
changing dirnode.py to use CachingDict
I've looked over your patches and it looks good! I'll work on my benchmarks of dirnode now so I can see what difference it makes to CPU usage.
I noticed when running a test script that zooko had that I was throwing away *args and **kwargs in my CachingDict subclass. I'm attaching a patch that fixes that issue.
Attachment dict.2.txt (19958 bytes) added
(erm, not **kwargs -- typo)
changeset:e414c73877ba6337 adds benchmarking of unpack-followed-by-repack, which is the functionality that this ticket and Kevan's patch is about.
I've run the benchmarks, and am attaching the results.
To get the
unmodifed_results
file, I simply ranbench_dirnode.py
against an unmodified checkout from trunk.To get the
modified_results
file, I needed to change a call todict
inbench_dirnode.py
to refer todirnode.CachingDict
(otherwise_pack_contents
raises anAssertionError
). I'm attaching a patch that duplicates that change. I then applieddict.txt
,optimizations.txt
, andtests.txt
to an otherwise unmodified source tree, and ranbench_dirnode.py
against it.The results seem to favor the optimized implementation.
I'm also updating dict.txt again -- hopefully for the last time -- to fix another *args related issue that I found.
Attachment dict.txt (19947 bytes) added
CachingDict implementation
Attachment bench.txt (20384 bytes) added
benchmark modifications for testing optimized code
Attachment modified_benchmark (1977 bytes) added
Benchmarks for optimized code
Attachment unmodified_benchmark (1977 bytes) added
benchmarks for unoptimized code
could you summarize the benchmark numbers in terms of A+Bx factors? Also, it's not clear to me what those numbers point to as being the expensive part: is it the netstring parsing? or the encryption?
Summarizing into
A+Bx
is not the way I do it. Instead I look at a handful of data points, like this:This is a good example of why I do it this way: because it isn't linear! So any
A+Bx
summary would be off. I already have a patch which fixes the observable non-linearity there. I guess I might as well commit that one so that everyone can see it. There -- changeset:efafcfb91a09b4de, entitled "directories: keep track of your position as you decode netstring after netstring from an input buffer instead of copying the trailing part | This makes decoding linear in the number of netstrings instead of O(N^2^).". After this patch, the same benchmark on the same machine says:As to your question about what's the expensive part, after the optimization patch from Kevan plus a few that I have here in my sandbox, benchmarking shows:
and profiling shows:
Whoops, time for me to go to work! I hope to measure and commit the rest of my optimization patches today after work. There is one that I really want other people's input on before I commit it... That one generates the per-entry IV with a secure hash of the writecap instead of with
os.urandom(16)
.Fixed by changeset:903005a52830ba96 but see also the new #752 (speed up directories more). Thanks, Kevan!
Note that r3971 didn't actually commit the optimization code -- just the dictionary that it uses. If you want that, you should also apply the optimizations.txt patch and the tests.txt patch (which also contains tests for CachingDict).
Oh how embarassing that I thought that was the optimization patch. I was actually going to ask you what you thought about the mysterious fact that changeset:903005a52830ba96 seems to moderately speed up pack and unpack but not so much unpack-and-repack!
Now that I've applied your actual optimization patch, time to unpack and repack a 4096-entry directory dropped substantially. However, time to pack and time to unpack also changed, which suggests that my benchmark script is accidentally using your cache, or that extraneous factors on my system are screwing up the results or something.
Before
optimizations.txt
:after
optimizations.txt
:I've applied your patches plus one from me to fix a conflict between one of my optimizations and one of yours.
The patch to cache entries was and required follow-up merge patch changeset:34213cd2c70246f3.
At around the same time other patches to optimize directory processing were also committed: changeset:efafcfb91a09b4de (the big one that makes processing linear instead of O(N^2^)), changeset:c0d1e7deaec145d6, changeset:786ed012b3510135 (which required urgent security fix follow-up changeset:c1d5717cf0ecd68f.