Commit Graph

15 Commits

Author SHA1 Message Date
Jun Wu
cd2f9f4715 cdatapack: do not keep fd open
Once `mmap` succeeded, it's no longer necessary to keep the underlying fd
open. Therefore just close them as an attempt to reduce file descriptors
opened by the process.

Thanks for spectral from Google for pointing out that fd could be closed.

Test Plan:
```
make clean local
rt test-cstore.t test-*pack*.t
```

Differential Revision: https://phab.mercurial-scm.org/D1185
2017-10-18 16:52:58 -07:00
Jun Wu
114719fb15 cdatapack: move feature macros before including cdatapack.h
On Arch Linux, glibc 2.25-7, `cdatapack.h` includes `stdint.h`, which
includes `bits/libc-header-start.h`, which includes `features.h`. Therefore
`_DEFAULT_SOURCE` and `_BSD_SOURCE` must be defined before including
`cdatapack.h`.

`ntoh_data_offset` was moved to `.c` to make sure `.c` is the only direct
and indirect user of `be64toh`.

Test Plan:
Make sure `make local` works on Arch Linux.

Differential Revision: https://phab.mercurial-scm.org/D559
2017-08-30 13:17:07 -07:00
Adam Simpkins
5afbd505ef clib: clean up include guards in the C/C++ header files
Summary:
Clean up the include guards to be more consistent and unique.

Some files used include guards like "KEY_H" and "STORE_H" which were not very
unique, and are more likely to collide with definitions provided by header
files from other projects.  Some of the py-*.h files were missing include
guards altogether.

This corresponds to Facebook diff D5588670.

Test Plan: Confirmed the code still builds and passes tests.

Reviewers: #fbhgext, quark

Reviewed By: #fbhgext, quark

Differential Revision: https://phab.mercurial-scm.org/D508
2017-08-25 16:46:07 -07:00
Adam Simpkins
ecb0fd2dd7 clib: update C/C++ copyright statements to pass lint checks
Summary:
Update the copyright headers in most of the C/C++ code consistently use the
GPLv2 copyright message.  This allows these files to pass Facebook's internal
C/C++ linters.

Some of the files in fbcode/scm/hgext/cstore/ appear to have actually been
copied from the hg-crew repository, and were not originally authored by
Facebook.  I have not modified the copyright statements in these files:

- cstore/bitmanipulation.h
- cstore/compat.h
- cstore/mpatch.h
- cstore/mpatch.c

I also have not modified any of the cfastmanifest code.

This corresponds to Facebook diff D5588677.

Test Plan:
Confirmed that Facebook's C++ linters no longer complain about the copyright
messages.

Reviewers: #fbhgext, quark

Reviewed By: #fbhgext, quark

Differential Revision: https://phab.mercurial-scm.org/D507
2017-08-25 16:46:07 -07:00
Adam Simpkins
b67af2812c clib: simplify include paths in C extensions
Summary:
Convert the C and C++ files in cdatapack, clib, cstore, and ctreemanifest
to always include files from the root of fb-hgext.  This simplifies the build
process by no long requiring a variety of separate include directories to be
specified on the compiler command line.

This will also make it easier to re-use these extensions in other projects
with different build systems.

This corresponds to the Facebook diff D5588676.

Test Plan: Confirmed that "make local" succeeds from a clean build.

Reviewers: #fbhgext, quark

Reviewed By: #fbhgext, quark

Differential Revision: https://phab.mercurial-scm.org/D505
2017-08-25 16:46:07 -07:00
Jun Wu
a63cbaaed1 portibility: move to clib
Summary:
It's a small C utility. It should belong to `clib` directory.

Also, `#include "../foo"` does not seem to be a good pattern. It makes
include files harder to follow and make code movement more difficult. Since
`clib` is already included in `-I` during compilation, remove `../`.

Test Plan: `make clean local`. It still builds.

Reviewers: ikostia, #mercurial, rmcelroy

Reviewed By: rmcelroy

Subscribers: mjpieters, vsutaria

Differential Revision: https://phabricator.intern.facebook.com/D5113236

Signature: t1:5113236:1495562436:e8d64083ab0417c67b63223a092470739f4c1176
2017-05-23 11:47:40 -07:00
Jun Wu
2ad4e3a191 cdatapack: avoid lz4decompress in getdeltachainlink
Summary:
This patch moves lz4decompress logic to a separate function from
getdeltachainlink.

This should solve a memory leak issue and speed up datapack entry iteration.
Practically, this means repack will be faster and take less memory.

Test Plan: `make clean local`, run `test-cstore.t`, and `test-remotefilelog-repack-fast.t`

Reviewers: #mercurial, durham

Reviewed By: durham

Subscribers: mjpieters, terrelln

Differential Revision: https://phabricator.intern.facebook.com/D4960035

Signature: t1:4960035:1493609520:a3c74bae92b8fff85ccadd9dd412a0c2b05573ac
2017-05-01 13:03:38 -07:00
Jun Wu
4240bd017e remotefilelog: let content stores support metadata
Summary:
This diffs add a `getmeta` method to all content stores. The cdatapack code is
modified to pass the tests, it needs further change to support `getmeta`.

The datapack format is bumped to v1 from v0. For v1, we append a `metadata`
dict at the end of each revision. The dict is currently used to store revlog
flags and rawsize of raw revlog fulltext. In the future we can put more data
like a second hash etc, without changing API or format again.

This diff focuses on correctness. A datapack caching layer to speed up
`getmeta` will be added later.

Tests are updated since we write new v1 packfile now and the format change
leads to different content and packfile names.

`Makefile`, `ls-l.py` are added to make tests easier to maintain.

Test Plan: Updated existing tests.

Reviewers: #mercurial, rmcelroy, durham

Reviewed By: durham

Subscribers: rmcelroy, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4903917

Signature: t1:4903917:1493255844:7ef5d487096cd2f78f2aaae672a68d49f33632ee
2017-04-26 19:50:36 -07:00
Wez Furlong
4a59f3b701 c-extensions: fixup some compiler/environment portability concerns
Summary:
I sync'd a copy of this code into the eden repository.
I had to adjust a couple of include paths to get the code to
compile correctly in the hermetic build environment that is
in use there.

In addition, our linter suite over there found a couple of C++ nits
to be fixed up.

Test Plan: make local

Reviewers: simpkins, ikostia, simonfar, durham

Reviewed By: durham

Subscribers: net-systems-diffs@fb.com, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4879285

Signature: t1:4879285:1492039044:8cb1e033e35ee568806de94dda3d2f6f8e78f5cb
2017-04-12 16:34:53 -07:00
Kostia Balytskyi
67b7f56ddb portability: add a portability header
Summary:
Proposed header (or its dir) is a single place to put MSVC/GCC hacks. So
far it only includes the COMPOUND_LITERAL macro which behaves differently
depending on MSVC mode.
When MSVC2015 is used in C++ mode, it does not support things like:
`(my_type) {initializers}`, but in C mode it does.

To clarify: I am not even sure whether we need to have the ability to compile in a purely C mode, but I did not want to figure out.

Test Plan: - on Linux, run `python setup.py build`, run all the tests, see them passing

Reviewers: #sourcecontrol, tja

Reviewed By: tja

Subscribers: tja, jsgf, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4843230

Signature: t1:4843230:1491496062:3fa10ae5a5aac850689991de1ca6ee1ac86d9dce
2017-04-06 09:33:34 -07:00
Durham Goode
4fd00d751a cstore: C++ implementation of datapackstore
Summary:
The remaining python parts of the store are a perf bottleneck when accessing
hundreds of thousands of pack file entries (like in treemanifest). Let's
implement them in C++.

This first patch just add the basic boiler plate, and implements a single
function getdeltachain(), with a test. Future patches will add more
functionality and other parts of the store.

Since cstore depends on cdatapack and ctreemanifest (the pythonutils.h part for
now), we need to tweak our setup.py to enforce a certain build order too.

Test Plan: Added a test, yo

Reviewers: #mercurial, simonfar

Reviewed By: simonfar

Subscribers: simonfar, stash, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4547929

Signature: t1:4547929:1487181318:21c146cf370d26cb97efe6a883868b85b4e32f49
2017-02-23 14:03:03 -08:00
Durham Goode
a808c980f0 Backed out changeset c84de4b54530
The cstore changes are breaking the build in some unusual ways and I will need
some time to fix them. Let's back it out for now.
2017-02-16 14:37:23 -08:00
Durham Goode
41486c3f47 cstore: C++ implementation of datapackstore
Summary:
The remaining python parts of the store are a perf bottleneck when accessing
hundreds of thousands of pack file entries (like in treemanifest). Let's
implement them in C++.

This first patch just add the basic boiler plate, and implements a single
function getdeltachain(), with a test. Future patches will add more
functionality and other parts of the store.

Since cstore depends on cdatapack and ctreemanifest (the pythonutils.h part for
now), we need to tweak our setup.py to enforce a certain build order too.

Test Plan: Added a test, yo

Reviewers: #mercurial, simonfar

Reviewed By: simonfar

Subscribers: simonfar, stash, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4547929

Signature: t1:4547929:1487181318:21c146cf370d26cb97efe6a883868b85b4e32f49
2017-02-15 15:19:36 -08:00
Augie Fackler
22653c7629 cdatapack: include sys/types.h for off_t
This isn't required on all systems, but I ran into one where it
was. Sigh. I've struggled a bit to find proper documentation of this,
and this is the best I could do:
http://pubs.opengroup.org/onlinepubs/009696799/basedefs/sys/types.h.html
2016-11-28 17:24:45 -05:00
Durham Goode
50d6b599f4 Move ctreemanifest and cdatapack out of remotefilelog
These don't really have any dependencies on remotefilelog, so let's move them
out.
2016-09-21 13:55:12 -07:00