feat: add parallel cache support #7131

keradus · 2023-07-09T00:18:31Z

No description provided.

keradus · 2023-07-10T08:39:14Z

src/Cache/FileHandler.php

+            return;
+        }
+
+        if (getenv('PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE')) {


idea for future (out of this PR scope) - we can detect that file was modified since it was read initially, and do the backilling based on that indicator, without env var.

implemented in 2aef0f8

ker@d ~/github/PHP-CS-Fixer λ ./php-cs-fixer list-files --config=.php-cs-fixer.dist.php | xargs -n 400 -P 3 ./php-cs-fixer fix --config=.php-cs-fixer.dist.php --path-mode intersection Loaded config default from ".php-cs-fixer.dist.php". Using cache file ".php-cs-fixer.cache". Loaded config default from ".php-cs-fixer.dist.php". Using cache file ".php-cs-fixer.cache". Loaded config default from ".php-cs-fixer.dist.php". Using cache file ".php-cs-fixer.cache". string(34) " 73993 !!! FRS !!! READ 1689350710" string(34) " 73994 !!! FRS !!! READ 1689350710" string(34) " 73995 !!! FRS !!! READ 1689350710" Fixed 0 of 212 files in 7.380 seconds, 20.000 MB memory used string(31) " 73995 !!! FRS !!! DECISION ???" array(3) { ["property"]=> int(1689350710) ["os"]=> int(1689350710) ["should backfill"]=> bool(false) } string(28) " 73995 !!! FRS !!! SAVE FILE" string(40) " 73995 !!! FRS !!! AFTER SAVE 1689350727" Fixed 0 of 400 files in 23.354 seconds, 22.000 MB memory used string(31) " 73993 !!! FRS !!! DECISION ???" array(3) { ["property"]=> int(1689350710) ["os"]=> int(1689350727) ["should backfill"]=> bool(true) } string(27) " 73993 !!! FRS !!! BACKFILL" string(28) " 73993 !!! FRS !!! SAVE FILE" string(40) " 73993 !!! FRS !!! AFTER SAVE 1689350743" string(31) " 73994 !!! FRS !!! DECISION ???" array(3) { ["property"]=> int(1689350710) ["os"]=> int(1689350743) ["should backfill"]=> bool(true) } string(27) " 73994 !!! FRS !!! BACKFILL" string(28) " 73994 !!! FRS !!! SAVE FILE" string(40) " 73994 !!! FRS !!! AFTER SAVE 1689350748" Fixed 0 of 400 files in 27.835 seconds, 22.000 MB memory used

Wirone

I am not sure if there should be internal logic for handling parallelisation in cache, when parallelisation itself is possible only using external tooling and we can't determine if it's used or not in current run.

Wirone · 2023-07-10T09:34:09Z

doc/usage.rst

+    export PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE=1
    php php-cs-fixer.phar list-files --config=.php-cs-fixer.dist.php | xargs -n 10 -P 8 php php-cs-fixer.phar fix --config=.php-cs-fixer.dist.php --path-mode intersection -v


Suggested change

export PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE=1

php php-cs-fixer.phar list-files --config=.php-cs-fixer.dist.php | xargs -n 10 -P 8 php php-cs-fixer.phar fix --config=.php-cs-fixer.dist.php --path-mode intersection -v

PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE=1 php php-cs-fixer.phar list-files --config=.php-cs-fixer.dist.php | xargs -n 10 -P 8 php php-cs-fixer.phar fix --config=.php-cs-fixer.dist.php --path-mode intersection -v

IMHO it shouldn't be exported, but defined only for this particular run, that is supported by external parallelisation. Exporting will keep that value and will use that feature also for regular run (without xargs) and I am not sure if it should be like this.

var_dump(">>>", getenv('PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE')); die;

your proposal is returning

string(3) ">>>" bool(false)

some playground

ker@d ~/github/PHP-CS-Fixer λ cat x.php <?php echo json_encode([basename(__FILE__) => getenv('ENV_FOO')]);

ker@d ~/github/PHP-CS-Fixer λ cat y.php <?php echo json_encode([basename(__FILE__) => getenv('ENV_FOO')]);

ker@d ~/github/PHP-CS-Fixer λ ENV_FOO=TEST php x.php {"x.php":"TEST"}

ker@d ~/github/PHP-CS-Fixer λ ENV_FOO=TEST php x.php | php y.php {"y.php":false}

I did not thought about pipes and variable visibility. It should be something like FOO=bar bash -c 'somecommand someargs | somecommand2' (source), in this case variable is available on each level of the pipeline. It makes it a little bit more complicated, but enables feature for single run instead of doing it globally. I won't fight for it, just a suggestion 🙂.

i think it makes it overcomplex and I'm ok to have env exported. I assume that one either will want it each time, or never - and if they need sometimes, they will figure out the way.

bash is also less portable

solved in #7131 (comment)

doc/usage.rst

ker@d ~/github/PHP-CS-Fixer λ ./php-cs-fixer list-files --config=.php-cs-fixer.dist.php | xargs -n 400 -P 3 ./php-cs-fixer fix --config=.php-cs-fixer.dist.php --path-mode intersection Loaded config default from .php-cs-fixer.dist.php. Using cache file .php-cs-fixer.cache. Loaded config default from .php-cs-fixer.dist.php. Using cache file .php-cs-fixer.cache. Loaded config default from .php-cs-fixer.dist.php. Using cache file .php-cs-fixer.cache. string(34)

keradus · 2023-07-14T16:27:35Z

src/Cache/FileHandler.php

+        }
+        var_dump("\n".getmypid().' !!! FRS !!! SAVE FILE');
+        ftruncate($handle, 0);
+        fwrite($handle, $cache->toJson());


I wonder about putting @ to silence warning and check !== false for every method here... any better suggestion?

This reverts commit 10f2de7.

src/Cache/FileHandler.php

keradus · 2023-10-16T10:35:36Z

go away stale bot, we look for brave reviewer

coveralls · 2023-10-16T11:05:43Z

coverage: 94.603% (-0.002%) from 94.605% when pulling 3b4dc19 on keradus:PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE into 7f0f15e on PHP-CS-Fixer:master.

…RIMENTAL_PARALLEL_CACHE

Wirone · 2023-10-16T11:33:50Z

go away stale bot, we look for brave reviewer

I don't review PRs with CI failures 😆. Unless it's PoC and I'm asked for the feedback 😉.

keradus · 2023-10-16T11:53:14Z

Not saying who would be that reviewer ;)

We were discussing idea internally and I opened it as POC, if we conclude on value to finish it (or not) - #7131 (review) - happy to continue.
Also, still looking for some better suggestions around #7131 (comment) :(

But not worth to continue with everything technical to conclude "nah, we do not like the idea itself".

keradus · 2023-10-16T12:02:02Z

Also, it was green before you asked for solving this
#7131 (comment)

;)

keradus added 3 commits July 9, 2023 02:12

move logic to support functions

c6cadbc

PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE

1ba6e58

docs

10f2de7

keradus marked this pull request as ready for review July 9, 2023 00:18

keradus added 2 commits July 9, 2023 02:37

fix

5af2c0a

kick

d3cb705

keradus commented Jul 10, 2023

View reviewed changes

Wirone reviewed Jul 10, 2023

View reviewed changes

keradus commented Jul 10, 2023

View reviewed changes

doc/usage.rst Outdated Show resolved Hide resolved

keradus added 8 commits July 14, 2023 16:58

Merge branch 'master' into PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE

ea7219e

__________________

e06771d

remove debug

8cecc03

no need to get whole stat

30dec99

strict comparison

10560c6

store lastModification on write

29530e0

touch OS to backfill only if backfill method exists

6e52cd4

keradus commented Jul 14, 2023

View reviewed changes

keradus added 4 commits July 14, 2023 18:28

Revert "docs"

ac074e1

This reverts commit 10f2de7.

better naming 1

3c8e02c

Better naming 2

f0dbe27

less nesting

c0d1120

keradus force-pushed the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch from 3233885 to 4d67ada Compare July 14, 2023 16:39

fix reported phpstan issues

d25c533

keradus force-pushed the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch from 4d67ada to d25c533 Compare July 14, 2023 16:39

keradus changed the title ~~DX: add experimental parallel cache support~~ DX: add parallel cache support Jul 14, 2023

keradus commented Jul 15, 2023

View reviewed changes

src/Cache/FileHandler.php Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

github-actions bot added the status/stale label Oct 13, 2023

keradus removed the status/stale label Oct 16, 2023

Wirone force-pushed the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch from d25c533 to 6a70a58 Compare October 16, 2023 10:54

Merge remote-tracking branch 'upstream/master' into PHP_CS_FIXER_EXPE…

5b227da

…RIMENTAL_PARALLEL_CACHE

keradus force-pushed the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch from 6a70a58 to 5b227da Compare October 16, 2023 11:15

keradus added 2 commits October 28, 2023 13:59

Merge branch 'master' into PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE

860c4c0

Update src/Cache/FileHandler.php

c9ee48a

keradus force-pushed the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch from efd8135 to c9ee48a Compare October 28, 2023 12:03

keradus added 3 commits October 28, 2023 14:37

debug

efef110

handle errors

5f96e9d

revert debug

b22d37d

keradus changed the title ~~DX: add parallel cache support~~ feat: add parallel cache support Oct 28, 2023

more docs

98a2c72

keradus force-pushed the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch from 3b4dc19 to 98a2c72 Compare October 28, 2023 14:45

keradus merged commit 4ff6ed7 into PHP-CS-Fixer:master Oct 28, 2023
14 checks passed

keradus deleted the PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE branch October 28, 2023 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add parallel cache support #7131

feat: add parallel cache support #7131

keradus commented Jul 9, 2023

keradus Jul 10, 2023

keradus Jul 14, 2023

Wirone left a comment

Wirone Jul 10, 2023

keradus Jul 10, 2023 •

edited

Wirone Jul 10, 2023

keradus Jul 10, 2023 •

edited

keradus Jul 14, 2023

keradus Jul 14, 2023

This comment was marked as outdated.

keradus commented Oct 16, 2023

coveralls commented Oct 16, 2023 •

edited

Wirone commented Oct 16, 2023

keradus commented Oct 16, 2023 •

edited

keradus commented Oct 16, 2023

		export PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE=1
		php php-cs-fixer.phar list-files --config=.php-cs-fixer.dist.php \| xargs -n 10 -P 8 php php-cs-fixer.phar fix --config=.php-cs-fixer.dist.php --path-mode intersection -v

	export PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE=1
	php php-cs-fixer.phar list-files --config=.php-cs-fixer.dist.php \| xargs -n 10 -P 8 php php-cs-fixer.phar fix --config=.php-cs-fixer.dist.php --path-mode intersection -v
	PHP_CS_FIXER_EXPERIMENTAL_PARALLEL_CACHE=1 php php-cs-fixer.phar list-files --config=.php-cs-fixer.dist.php \| xargs -n 10 -P 8 php php-cs-fixer.phar fix --config=.php-cs-fixer.dist.php --path-mode intersection -v

feat: add parallel cache support #7131

feat: add parallel cache support #7131

Conversation

keradus commented Jul 9, 2023

keradus Jul 10, 2023

Choose a reason for hiding this comment

keradus Jul 14, 2023

Choose a reason for hiding this comment

Wirone left a comment

Choose a reason for hiding this comment

Wirone Jul 10, 2023

Choose a reason for hiding this comment

keradus Jul 10, 2023 • edited

Choose a reason for hiding this comment

Wirone Jul 10, 2023

Choose a reason for hiding this comment

keradus Jul 10, 2023 • edited

Choose a reason for hiding this comment

keradus Jul 14, 2023

Choose a reason for hiding this comment

keradus Jul 14, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

keradus commented Oct 16, 2023

coveralls commented Oct 16, 2023 • edited

Wirone commented Oct 16, 2023

keradus commented Oct 16, 2023 • edited

keradus commented Oct 16, 2023

keradus Jul 10, 2023 •

edited

keradus Jul 10, 2023 •

edited

coveralls commented Oct 16, 2023 •

edited

keradus commented Oct 16, 2023 •

edited