Skip to content

Conversation

@yangweijie
Copy link

only linux os need add DIRECTORY_SEPARATOR if path not begins with '/'

Change Log

Added

Fixed

  • path wrong in windows.

Changed

Removed

Deprecated

Security


Description

only linux os need add DIRECTORY_SEPARATOR if path not begins with '/'
@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2024

Flow PHP - Benchmarks

Results of the benchmarks from this PR are compared with the results from 1.x branch.

Extractors
+-----------------------+-------------------+------+-----+-----------------+------------------+----------------+
| benchmark             | subject           | revs | its | mem_peak        | mode             | rstdev         |
+-----------------------+-------------------+------+-----+-----------------+------------------+----------------+
| CSVExtractorBench     | bench_extract_10k | 1    | 3   | 4.619mb +0.01%  | 513.584ms -0.62% | ±0.12% -79.10% |
| JsonExtractorBench    | bench_extract_10k | 1    | 3   | 4.708mb +0.01%  | 1.088s +0.33%    | ±0.75% -21.24% |
| ParquetExtractorBench | bench_extract_10k | 1    | 3   | 29.315mb +0.00% | 437.507ms -1.50% | ±1.04% -48.19% |
| TextExtractorBench    | bench_extract_10k | 1    | 3   | 4.348mb +0.01%  | 33.204ms -1.33%  | ±0.99% +98.09% |
| XmlExtractorBench     | bench_extract_10k | 1    | 3   | 4.330mb +0.01%  | 651.032ms -0.37% | ±1.12% +16.91% |
+-----------------------+-------------------+------+-----+-----------------+------------------+----------------+
Transformers
+-----------------------------+--------------------------+------+-----+------------------+-----------------+----------------+
| benchmark                   | subject                  | revs | its | mem_peak         | mode            | rstdev         |
+-----------------------------+--------------------------+------+-----+------------------+-----------------+----------------+
| RenameEntryTransformerBench | bench_transform_10k_rows | 1    | 3   | 116.624mb +0.00% | 59.734ms -0.32% | ±0.57% +23.14% |
+-----------------------------+--------------------------+------+-----+------------------+-----------------+----------------+
Loaders
+--------------------+----------------+------+-----+------------------+------------------+----------------+
| benchmark          | subject        | revs | its | mem_peak         | mode             | rstdev         |
+--------------------+----------------+------+-----+------------------+------------------+----------------+
| CSVLoaderBench     | bench_load_10k | 1    | 3   | 54.817mb +0.00%  | 137.039ms -2.15% | ±0.84% +72.76% |
| JsonLoaderBench    | bench_load_10k | 1    | 3   | 90.401mb +0.00%  | 117.617ms +0.51% | ±0.68% -21.27% |
| ParquetLoaderBench | bench_load_10k | 1    | 3   | 124.671mb +0.00% | 1.243s -0.40%    | ±0.11% -80.83% |
| TextLoaderBench    | bench_load_10k | 1    | 3   | 17.539mb +0.00%  | 44.282ms -0.27%  | ±0.37% -60.92% |
+--------------------+----------------+------+-----+------------------+------------------+----------------+
Building Blocks
+-------------------------+----------------------------+------+-----+------------------+------------------+------------------+
| benchmark               | subject                    | revs | its | mem_peak         | mode             | rstdev           |
+-------------------------+----------------------------+------+-----+------------------+------------------+------------------+
| RowsBench               | bench_chunk_10_on_10k      | 2    | 3   | 87.372mb +0.00%  | 3.500ms +3.73%   | ±1.57% +272.78%  |
| RowsBench               | bench_diff_left_1k_on_10k  | 2    | 3   | 102.975mb +0.00% | 188.743ms +0.88% | ±0.78% +73.72%   |
| RowsBench               | bench_diff_right_1k_on_10k | 2    | 3   | 85.695mb +0.00%  | 18.720ms -0.16%  | ±0.33% +100.30%  |
| RowsBench               | bench_drop_1k_on_10k       | 2    | 3   | 88.612mb +0.00%  | 1.651ms -9.76%   | ±2.25% -13.12%   |
| RowsBench               | bench_drop_right_1k_on_10k | 2    | 3   | 88.612mb +0.00%  | 1.689ms -5.24%   | ±2.91% +351.05%  |
| RowsBench               | bench_entries_on_10k       | 2    | 3   | 85.724mb +0.00%  | 2.839ms +0.61%   | ±1.36% +6.19%    |
| RowsBench               | bench_filter_on_10k        | 2    | 3   | 86.253mb +0.00%  | 15.290ms -3.61%  | ±2.66% +27.71%   |
| RowsBench               | bench_find_on_10k          | 2    | 3   | 86.253mb +0.00%  | 14.928ms -4.85%  | ±2.59% +58.18%   |
| RowsBench               | bench_find_one_on_10k      | 10   | 3   | 84.156mb +0.00%  | 1.706μs -9.92%   | ±2.72% +7.69%    |
| RowsBench               | bench_first_on_10k         | 10   | 3   | 84.156mb +0.00%  | 0.400μs 0.00%    | ±0.00% 0.00%     |
| RowsBench               | bench_flat_map_on_1k       | 2    | 3   | 93.507mb +0.00%  | 13.033ms +2.15%  | ±0.56% -57.17%   |
| RowsBench               | bench_map_on_10k           | 2    | 3   | 122.878mb +0.00% | 63.091ms +1.78%  | ±0.90% +6.03%    |
| RowsBench               | bench_merge_1k_on_10k      | 2    | 3   | 86.772mb +0.00%  | 1.690ms +18.28%  | ±3.22% +523.76%  |
| RowsBench               | bench_partition_by_on_10k  | 2    | 3   | 90.124mb +0.00%  | 65.422ms +2.15%  | ±0.33% -70.34%   |
| RowsBench               | bench_remove_on_10k        | 2    | 3   | 88.874mb +0.00%  | 4.586ms +9.46%   | ±2.21% +1934.44% |
| RowsBench               | bench_sort_asc_on_1k       | 2    | 3   | 84.306mb +0.00%  | 40.586ms +0.20%  | ±1.15% +221.54%  |
| RowsBench               | bench_sort_by_on_1k        | 2    | 3   | 84.307mb +0.00%  | 41.457ms +0.64%  | ±1.01% -17.85%   |
| RowsBench               | bench_sort_desc_on_1k      | 2    | 3   | 84.306mb +0.00%  | 40.949ms -0.54%  | ±1.02% +15.45%   |
| RowsBench               | bench_sort_entries_on_1k   | 2    | 3   | 86.598mb +0.00%  | 7.456ms +1.53%   | ±0.75% +344.97%  |
| RowsBench               | bench_sort_on_1k           | 2    | 3   | 84.156mb +0.00%  | 29.017ms -2.97%  | ±1.48% +110.93%  |
| RowsBench               | bench_take_1k_on_10k       | 10   | 3   | 84.156mb +0.00%  | 13.717μs +3.09%  | ±1.59% +349.27%  |
| RowsBench               | bench_take_right_1k_on_10k | 10   | 3   | 84.156mb +0.00%  | 16.256μs +2.35%  | ±2.25% +151.84%  |
| RowsBench               | bench_unique_on_1k         | 2    | 3   | 102.976mb +0.00% | 196.273ms +2.82% | ±1.39% -54.82%   |
| TypeDetectorBench       | bench_type_detector        | 1    | 3   | 53.198mb +0.00%  | 399.697ms +0.31% | ±0.21% -85.96%   |
| TypeDetectorBench       | bench_type_detector        | 1    | 3   | 13.464mb +0.00%  | 80.441ms +0.61%  | ±0.57% +1084.50% |
| NativeEntryFactoryBench | bench_entry_factory        | 1    | 3   | 107.460mb +0.00% | 490.652ms -0.36% | ±0.71% +11.63%   |
| NativeEntryFactoryBench | bench_entry_factory        | 1    | 3   | 55.818mb +0.00%  | 247.988ms +0.34% | ±1.28% -2.47%    |
| NativeEntryFactoryBench | bench_entry_factory        | 1    | 3   | 14.656mb +0.00%  | 54.083ms +0.98%  | ±0.48% +56.34%   |
+-------------------------+----------------------------+------+-----+------------------+------------------+------------------+

@norberttech
Copy link
Member

hey @yangweijie sorry I'm not sure if I follow, could you elaborate a bit more on this one and maybe provide some tests that would cover the changes from this PR?

@yangweijie
Copy link
Author

If you use in windows , u can find the bug easyly.
image
image
with fixed by just one line code for platform judgement
image

@yangweijie
Copy link
Author

the path generated by path concat by runtime_path function in ThinkPHP framework. Acturely , The const string 'C:\git\tp\runtime\log\20241103.parquet' which a realpah for a file exists will take the same effect.

image

@norberttech norberttech added the bug label Dec 5, 2024
@norberttech
Copy link
Member

I'm on the fence about this one. To do it right we would need first:

  • setup a windows runtime on GitHub Actions
  • adjust tests and whole codebase for Windows
  • provide Windows Support

However, due to my limited availability, I do not see a possibility for me to work on that in the near future.
So to make it happen someone would need to sponsor that feature request or take care of the whole thing and become a maintainer responsible for the Windows support only.

In its current form and shape this PR can't be merged as:

  • existing tests are failing
  • there are no tests covering this fix
  • not a single test is executed on Windows runtime.

I'm closing this one for now but will keep Windows Support in the Roadmap in case anyone would be ever interested in working on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants