New issue
Advanced search Search tips

Issue 844476 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 30
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Implement readdirplus optimization in smbprovider

Project Member Reported by zentaro@chromium.org, May 18 2018

Issue description

Backport patches and integrate.
 
Owner: zentaro@chromium.org
Labels: -Pri-3 Pri-2
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6

commit 380a0ef6fcb6d2d8b4485d7aef110e107544fcf6
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Thu Jul 26 16:19:43 2018

net-fs/samba: Backport readdir plus optimization.

Backports an optimization that returns a combined read directory
with stat to return all metadata at once without have to do
roundtrips for each files.

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I0fef31b6a433c12cbc4ed513d4a96aa10fde4128
Reviewed-on: https://chromium-review.googlesource.com/1147303
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/samba-4.8.0.ebuild
[add] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/files/samba-4.8.0-readdirplus_3_add_dirplus.patch
[add] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/files/samba-4.8.0-readdirplus_4_update_abi.patch
[add] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/files/samba-4.8.0-readdirplus_2_data_structures.patch
[add] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/files/samba-4.8.0-readdirplus_1_btime.patch
[add] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/files/samba-4.8.0-readdirplus_5_cleanup_close.patch
[rename] https://crrev.com/380a0ef6fcb6d2d8b4485d7aef110e107544fcf6/net-fs/samba/samba-4.8.0-r7.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0fa814875328e4f8b33717dc246f491aadf08a1d

commit 0fa814875328e4f8b33717dc246f491aadf08a1d
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 06:59:43 2018

smbprovider: Add stubs for readdirplus

- Add GetDirectoryEntryWithMetadata to SambaInterface
- Add stub to the impl and fake

CQ-DEPEND=CL:1147303
BUG= chromium:844476 , chromium:757625 
TEST=emerges and unittests

Change-Id: I3a4eb42bf30da4bc033c8f67f8038bbe9c4a575c
Reviewed-on: https://chromium-review.googlesource.com/1104991
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/0fa814875328e4f8b33717dc246f491aadf08a1d/smbprovider/fake_samba_interface.h
[modify] https://crrev.com/0fa814875328e4f8b33717dc246f491aadf08a1d/smbprovider/fake_samba_interface.cc
[modify] https://crrev.com/0fa814875328e4f8b33717dc246f491aadf08a1d/smbprovider/samba_interface.h
[modify] https://crrev.com/0fa814875328e4f8b33717dc246f491aadf08a1d/smbprovider/samba_interface_impl.cc
[modify] https://crrev.com/0fa814875328e4f8b33717dc246f491aadf08a1d/smbprovider/samba_interface_impl.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8fc1dd86b38d256a7589671cb09e1ae2430112de

commit 8fc1dd86b38d256a7589671cb09e1ae2430112de
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 06:59:43 2018

smbprovider: Add fake implementation of readdirplus

- Add fake implementation and tests

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Iebf0256d76401524e102e4c7024afdbf199776e9
Reviewed-on: https://chromium-review.googlesource.com/1104992
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/8fc1dd86b38d256a7589671cb09e1ae2430112de/smbprovider/fake_samba_interface.h
[modify] https://crrev.com/8fc1dd86b38d256a7589671cb09e1ae2430112de/smbprovider/fake_samba_interface.cc
[modify] https://crrev.com/8fc1dd86b38d256a7589671cb09e1ae2430112de/smbprovider/constants.h
[modify] https://crrev.com/8fc1dd86b38d256a7589671cb09e1ae2430112de/smbprovider/fake_samba_test.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/375ecd1ac23eeeeb5ce99ffb69918c466fd564b1

commit 375ecd1ac23eeeeb5ce99ffb69918c466fd564b1
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 06:59:44 2018

smbprovider: Add implementation of readdirplus

- Code is not called yet.
- Follow up CL's integrate to SmbProvider

BUG= chromium:844476 , chromium:757625 
TEST=emerges and unittests

Change-Id: I943fd84b03788539b582762e807003e0cdfcbf02
Reviewed-on: https://chromium-review.googlesource.com/1104993
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/375ecd1ac23eeeeb5ce99ffb69918c466fd564b1/smbprovider/samba_interface_impl.h
[modify] https://crrev.com/375ecd1ac23eeeeb5ce99ffb69918c466fd564b1/smbprovider/samba_interface_impl.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/32cabe73b153e2b40a7a9fcc71db3efe14177e7c

commit 32cabe73b153e2b40a7a9fcc71db3efe14177e7c
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 15:50:54 2018

smbprovider: Add plumbing to iterators to support metadata

- Adds additional argument to BaseDirectoryIterator to specify
  whether the iterator should populate metadata while reading
  the directory.
- Makes no change to existing functionality in this CL.
- The new method to populate the metadata is implemented in a
  follow up CL.

BUG= chromium:844476 , chromium:757625 
TEST=emerges and unittest

Change-Id: I1658adf0fd3ede4366dcb5de8572cd737e46e489
Reviewed-on: https://chromium-review.googlesource.com/1104994
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/32cabe73b153e2b40a7a9fcc71db3efe14177e7c/smbprovider/iterator/directory_iterator.cc
[modify] https://crrev.com/32cabe73b153e2b40a7a9fcc71db3efe14177e7c/smbprovider/iterator/directory_iterator.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/033a847e3bb706164a0e02e4107cf60a5a698ec8

commit 033a847e3bb706164a0e02e4107cf60a5a698ec8
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 15:50:55 2018

smbprovider: Factor out a ClearVector() method

- Adds a helper method to clear the vector and reset the index
- Follow up CL will use this in an additional place

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Id4e96c0354d173a02a6a5b33438b8badea598582
Reviewed-on: https://chromium-review.googlesource.com/1104995
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/033a847e3bb706164a0e02e4107cf60a5a698ec8/smbprovider/iterator/directory_iterator.cc
[modify] https://crrev.com/033a847e3bb706164a0e02e4107cf60a5a698ec8/smbprovider/iterator/directory_iterator.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4148c30933a300ccc91c8daf8ce1d3fc5c4d9b42

commit 4148c30933a300ccc91c8daf8ce1d3fc5c4d9b42
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:33 2018

smbprovider: Implement iterator including metadata

- Adds implementation to call readdir plus to support an iterator
  that includes metadata.

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I3c1cd2f219ab5f0b42ea4d6f79ecddb4fa3496ba
Reviewed-on: https://chromium-review.googlesource.com/1104996
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/4148c30933a300ccc91c8daf8ce1d3fc5c4d9b42/smbprovider/iterator/directory_iterator_test.cc
[modify] https://crrev.com/4148c30933a300ccc91c8daf8ce1d3fc5c4d9b42/smbprovider/iterator/directory_iterator.cc
[modify] https://crrev.com/4148c30933a300ccc91c8daf8ce1d3fc5c4d9b42/smbprovider/iterator/directory_iterator.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0196479b97944850be6b1b027c2cd018b6d79922

commit 0196479b97944850be6b1b027c2cd018b6d79922
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:33 2018

smbprovider: Add skeleton of metadata cache.

- Adds do nothing cache class and test file
- Adds cache and test file to build
- Follow up CL implements the cache

BUG= chromium:844476 , chromium:757625 
TEST=emerges and unit tests

Change-Id: I05f0e2a19f51a184709f919f99b7b7e700c0b078
Reviewed-on: https://chromium-review.googlesource.com/1106770
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[add] https://crrev.com/0196479b97944850be6b1b027c2cd018b6d79922/smbprovider/metadata_cache.cc
[add] https://crrev.com/0196479b97944850be6b1b027c2cd018b6d79922/smbprovider/metadata_cache.h
[add] https://crrev.com/0196479b97944850be6b1b027c2cd018b6d79922/smbprovider/metadata_cache_test.cc
[modify] https://crrev.com/0196479b97944850be6b1b027c2cd018b6d79922/smbprovider/smbprovider.gyp

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4dd4469b76bae5009823921d7a523ec03d9f313a

commit 4dd4469b76bae5009823921d7a523ec03d9f313a
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:34 2018

smbprovider: Replace the mount_root with a MountInfo struct

- Changes the map in the mount manager to hold a struct instead
  of just a string.
- Currently the struct just contains the string mount_root field.
- Updates relevant references to the mount_root via mounts_
- Makes no change to behavior.
- Follow up CL will add the cache to the MountInfo struct.

BUG= chromium:844476 , chromium:757625 
TEST=emerges and unittests

Change-Id: Ib16706a3e90b683c17e8aa9a16fccb5ebce7eb34
Reviewed-on: https://chromium-review.googlesource.com/1106771
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/4dd4469b76bae5009823921d7a523ec03d9f313a/smbprovider/mount_manager.cc
[modify] https://crrev.com/4dd4469b76bae5009823921d7a523ec03d9f313a/smbprovider/mount_manager.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4b67f7a74545ec7869ececb35d913689bd750c51

commit 4b67f7a74545ec7869ececb35d913689bd750c51
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:37 2018

smbprovider: Wire up the metadata iterator

- The metadata iterator will now be used if caching is enabled
- The cache flag is currently off, so there is no behavior change
- Nothing currently consumes the additional metadata

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I70add04a16ace19ec3e8013398923ab802bad8ff
Reviewed-on: https://chromium-review.googlesource.com/1144613
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/4b67f7a74545ec7869ececb35d913689bd750c51/smbprovider/smbprovider.cc
[modify] https://crrev.com/4b67f7a74545ec7869ececb35d913689bd750c51/smbprovider/smbprovider.h
[modify] https://crrev.com/4b67f7a74545ec7869ececb35d913689bd750c51/smbprovider/smbprovider_test.cc
[modify] https://crrev.com/4b67f7a74545ec7869ececb35d913689bd750c51/smbprovider/iterator/directory_iterator.h
[modify] https://crrev.com/4b67f7a74545ec7869ececb35d913689bd750c51/smbprovider/constants.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/38ea0ba4104031e3386f8ba86c1e11cb40598cd1

commit 38ea0ba4104031e3386f8ba86c1e11cb40598cd1
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:37 2018

smbprovider: Add the cache to the MountManager

- Adds an instance of the cache for each mount in the mount
  manager.
- Nothing currently reads or writes from the cache.

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Id92d36c11d58684ea92823b56a5021d462237c91
Reviewed-on: https://chromium-review.googlesource.com/1144614
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/38ea0ba4104031e3386f8ba86c1e11cb40598cd1/smbprovider/mount_manager.cc
[modify] https://crrev.com/38ea0ba4104031e3386f8ba86c1e11cb40598cd1/smbprovider/metadata_cache.h
[modify] https://crrev.com/38ea0ba4104031e3386f8ba86c1e11cb40598cd1/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/38ea0ba4104031e3386f8ba86c1e11cb40598cd1/smbprovider/mount_manager.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3c1120159bee8430abea92f5f1605bcf1bbc08fe

commit 3c1120159bee8430abea92f5f1605bcf1bbc08fe
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:38 2018

smbprovider: Add code to check the cache on metadata requests

- Nothing populates the cache yet
- The flag is never turned on so this isn't run yet
- No change to existing behavior

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I4c329325ec7a45ffdca688afba28466e71a19c0d
Reviewed-on: https://chromium-review.googlesource.com/1144615
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/3c1120159bee8430abea92f5f1605bcf1bbc08fe/smbprovider/smbprovider.cc
[modify] https://crrev.com/3c1120159bee8430abea92f5f1605bcf1bbc08fe/smbprovider/smbprovider.h
[modify] https://crrev.com/3c1120159bee8430abea92f5f1605bcf1bbc08fe/smbprovider/smbprovider_test.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0781df5fffecf218a381480066a277fca7465fa4

commit 0781df5fffecf218a381480066a277fca7465fa4
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:38 2018

smbprovider: Implement a caching iterator

- Wraps an inner iterator and adds to the cache each time Get()
  is called

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I00aa5ed75f88cdd664b9455748f2f158ffbee055
Reviewed-on: https://chromium-review.googlesource.com/1144616
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[add] https://crrev.com/0781df5fffecf218a381480066a277fca7465fa4/smbprovider/iterator/caching_iterator.h
[modify] https://crrev.com/0781df5fffecf218a381480066a277fca7465fa4/smbprovider/smbprovider.gyp
[add] https://crrev.com/0781df5fffecf218a381480066a277fca7465fa4/smbprovider/iterator/caching_iterator_test.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9f221ba0bedef0a024f775901f1d7bd4b9728b8c

commit 9f221ba0bedef0a024f775901f1d7bd4b9728b8c
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:39 2018

smbprovider: Wire up CachingIterator in ReadDirectory

- When caching is enabled, ReadDirectory will use a caching
  iterator for DirectoryReads
- Caching is not used for reading shares.

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I8ed2d08354e08536d62c380b7ecfaaec8f741933
Reviewed-on: https://chromium-review.googlesource.com/1144617
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/9f221ba0bedef0a024f775901f1d7bd4b9728b8c/smbprovider/smbprovider.cc
[modify] https://crrev.com/9f221ba0bedef0a024f775901f1d7bd4b9728b8c/smbprovider/smbprovider_test.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7fd728f2f97bd6f0c527a9754b3a19e8edc78045

commit 7fd728f2f97bd6f0c527a9754b3a19e8edc78045
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:39 2018

smbprovider: Add a TickClock to MountManager

- Adds a TickClock to MountManager to be used for the metadata cache
- Currently the clock is unused
- Future CL will use the clock when constructing MetadataCache
  so that it can be used to invalidate cache entries
- Makes no change to current behavior

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I7b6d6bb9d1b7050add5569ca8c0366a20a8344fd
Reviewed-on: https://chromium-review.googlesource.com/1144618
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/7fd728f2f97bd6f0c527a9754b3a19e8edc78045/smbprovider/mount_manager.cc
[modify] https://crrev.com/7fd728f2f97bd6f0c527a9754b3a19e8edc78045/smbprovider/mount_manager.h
[modify] https://crrev.com/7fd728f2f97bd6f0c527a9754b3a19e8edc78045/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/7fd728f2f97bd6f0c527a9754b3a19e8edc78045/smbprovider/smbprovider_main.cc
[modify] https://crrev.com/7fd728f2f97bd6f0c527a9754b3a19e8edc78045/smbprovider/smbprovider_test.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921

commit dcfcf79752edb3624a4ee5c492cad5e6a6ec7921
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:40 2018

smbprovider: Adds TickClock to MetadataCache

- Construct the MetadataCache using the TickClock owned
  by MountManager
- Update test code
- Clock is currently not used
- Makes no change to current behavior

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Ibf245e518e6f240dd5f259b74d05346fd016b96a
Reviewed-on: https://chromium-review.googlesource.com/1144619
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921/smbprovider/mount_manager.cc
[modify] https://crrev.com/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921/smbprovider/metadata_cache_test.cc
[modify] https://crrev.com/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921/smbprovider/iterator/caching_iterator_test.cc
[modify] https://crrev.com/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921/smbprovider/metadata_cache.cc
[modify] https://crrev.com/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921/smbprovider/metadata_cache.h
[modify] https://crrev.com/dcfcf79752edb3624a4ee5c492cad5e6a6ec7921/smbprovider/mount_manager.h

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ecaf8e55bd2f641e18eb555879b344a2a4dc97ba

commit ecaf8e55bd2f641e18eb555879b344a2a4dc97ba
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:41 2018

smbprovider: Rename a genericly named variable

- kBufferSize gave an ambiguous name resolution
- Limit the scope of the variable and give it a better name

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Ie696151624d1bb34057e9dc945f1823b6d146547
Reviewed-on: https://chromium-review.googlesource.com/1144620
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/ecaf8e55bd2f641e18eb555879b344a2a4dc97ba/smbprovider/mount_manager_test.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 27

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/48f019d46479516677f5c80e467e1f7203f2eba7

commit 48f019d46479516677f5c80e467e1f7203f2eba7
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:42 2018

smbprovider: Support removing items from the metadata cache

- Adds method to remove an item from the cache by path
- Adds tests for removing item
- Does not change current behavior

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Ia2cc6c45e28351694f5d0beef092587002677410
Reviewed-on: https://chromium-review.googlesource.com/1144622
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/48f019d46479516677f5c80e467e1f7203f2eba7/smbprovider/metadata_cache.cc
[modify] https://crrev.com/48f019d46479516677f5c80e467e1f7203f2eba7/smbprovider/metadata_cache.h
[modify] https://crrev.com/48f019d46479516677f5c80e467e1f7203f2eba7/smbprovider/metadata_cache_test.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ff17c09d21a914380be0a7c9103ee838d04b2a9e

commit ff17c09d21a914380be0a7c9103ee838d04b2a9e
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:42 2018

smbprovider: Add an IsExpired method to MetadatCache

- Add private method to test if a cache entry has expired

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: If328b84b37298d20a5a1a3ba8731aa9d82148ab0
Reviewed-on: https://chromium-review.googlesource.com/1144623
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/ff17c09d21a914380be0a7c9103ee838d04b2a9e/smbprovider/metadata_cache.cc
[modify] https://crrev.com/ff17c09d21a914380be0a7c9103ee838d04b2a9e/smbprovider/metadata_cache.h

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9989a19c531171a5ac2f9f4098b59d6e61f4f482

commit 9989a19c531171a5ac2f9f4098b59d6e61f4f482
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:43 2018

smbprovider: FindEntry in cache respects expiration

- If an entry is expired don't return it and delete it

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: Ie126f51f3309c4c04fe9bfbcbe2e0e5e81b2b658
Reviewed-on: https://chromium-review.googlesource.com/1144624
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/9989a19c531171a5ac2f9f4098b59d6e61f4f482/smbprovider/metadata_cache.cc
[modify] https://crrev.com/9989a19c531171a5ac2f9f4098b59d6e61f4f482/smbprovider/metadata_cache.h
[modify] https://crrev.com/9989a19c531171a5ac2f9f4098b59d6e61f4f482/smbprovider/metadata_cache_test.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/bc4715434d4b9db305c784016983824202f9ce75

commit bc4715434d4b9db305c784016983824202f9ce75
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:43 2018

smbprovider: Support purging the entire cache by time

- Add a method to iterate the cache and remove items that are
  expired
- Makes no change to current behavior

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I18b2472069420c955a31a0107b0ee9aa16e1e04e
Reviewed-on: https://chromium-review.googlesource.com/1144625
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/bc4715434d4b9db305c784016983824202f9ce75/smbprovider/metadata_cache.cc
[modify] https://crrev.com/bc4715434d4b9db305c784016983824202f9ce75/smbprovider/metadata_cache.h
[modify] https://crrev.com/bc4715434d4b9db305c784016983824202f9ce75/smbprovider/metadata_cache_test.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/566801eb4d612aa4dfa94dbef020dc03368abfd6

commit 566801eb4d612aa4dfa94dbef020dc03368abfd6
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:44 2018

smbprovider: Purge the cache before each read directory

- Purges the cache of expired entries before each new directory
  is read
- This is the most simplistic strategy to keep the cache
  from growing uncontrollably
- Future CL's may further optimize this to use other information
  to decide whether to actually purge

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I7a986bf41d97cf20551397cfd34210fa6b64ed04
Reviewed-on: https://chromium-review.googlesource.com/1145554
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/566801eb4d612aa4dfa94dbef020dc03368abfd6/smbprovider/smbprovider.cc
[modify] https://crrev.com/566801eb4d612aa4dfa94dbef020dc03368abfd6/smbprovider/smbprovider_test.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/91be32d59fc705aa9d6892dcb57992ff84e961a2

commit 91be32d59fc705aa9d6892dcb57992ff84e961a2
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:44 2018

smbprovider: Enable the metadata cache

- Just turns on the caching flag to enable the optimization
- See previous CL's for full implementation
- Now all explicit ReadDirectories use readdirplus which reads
  directory entries with all the stat() metadata from one call
- The iterator caches the entries as they are iterated over
- GetMetadata calls check the cache before making a network call
- Cache just has a simple expiration time of 30 sec
- Cache is purged of all expired entries each time a new directory
  is read
- Future optimizations may make the purging smarter
- Implicit readDirectorys such as finding shares, or traversals
  to perform recursive copies or deletes do not use readdirplus
  or cache as they iterate

BUG= chromium:844476 , chromium:757625 
TEST=emerges

Change-Id: I07f0b37865a5ebb1a0f879f2b899aa73f7aa7f71
Reviewed-on: https://chromium-review.googlesource.com/1145555
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/91be32d59fc705aa9d6892dcb57992ff84e961a2/smbprovider/smbprovider_main.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a917d920050ae82b2be8f188a502983c550a6d07

commit a917d920050ae82b2be8f188a502983c550a6d07
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:35 2018

smbprovider: Support clearing and testing cache is empty

- Adds ClearAll() to remove all cache entries
- Adds IsEmpty() to test if the cache is empty

BUG= chromium:844476 , chromium:757625 
TEST=emerges and unittests

Change-Id: I125110aa29e93f1cd37360695325618640734851
Reviewed-on: https://chromium-review.googlesource.com/1146761
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/a917d920050ae82b2be8f188a502983c550a6d07/smbprovider/metadata_cache.cc
[modify] https://crrev.com/a917d920050ae82b2be8f188a502983c550a6d07/smbprovider/metadata_cache.h
[modify] https://crrev.com/a917d920050ae82b2be8f188a502983c550a6d07/smbprovider/metadata_cache_test.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e5a6f33596d96c402d25a418b50e27c74dcc923a

commit e5a6f33596d96c402d25a418b50e27c74dcc923a
Author: Zentaro Kavanagh <zentaro@chromium.org>
Date: Fri Jul 27 22:28:36 2018

smbprovider: Add support for setting date on fake directories

- Needed for testing the new metadata versions of ReadDirectory
  populate the fields correctly.

BUG= chromium:844476 , chromium:757625 
TEST=emerges and unittests

Change-Id: If128b4bb726ccb87a8112e8b668150a571b79903
Reviewed-on: https://chromium-review.googlesource.com/1146762
Commit-Ready: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/e5a6f33596d96c402d25a418b50e27c74dcc923a/smbprovider/fake_samba_interface.h
[modify] https://crrev.com/e5a6f33596d96c402d25a418b50e27c74dcc923a/smbprovider/fake_samba_interface.cc
[modify] https://crrev.com/e5a6f33596d96c402d25a418b50e27c74dcc923a/smbprovider/fake_samba_test.cc

Status: Fixed (was: Started)
Labels: M-70

Sign in to add a comment